Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for non-0 address spaces #877

Open
arsenm opened this issue Jan 6, 2023 · 2 comments
Open

Add support for non-0 address spaces #877

arsenm opened this issue Jan 6, 2023 · 2 comments
Labels
enhancement New feature or request memory Memory Model

Comments

@arsenm
Copy link
Contributor

arsenm commented Jan 6, 2023

define i32 @src(ptr addrspace(1) %ptr) {
  %load = load i32, ptr addrspace(1) %ptr
  ret i32 %load
}

define i32 @tgt(ptr addrspace(1) %ptr) {
  %load = load i32, ptr addrspace(1) %ptr
  ret i32 %load
}

ERROR: Could not translate 'src' to Alive IR

Seems to give up on any pointer value with a non-0 address space, no matter what you're doing with it. Care needs to be taken for null handling, but address spaces (except for non-integral address spaces) aren't total magic and should behave normally.

Currently the opt plugin has little practical use for AMDGPU since optimized IR has few addrspace(0) pointers. Manually fixing up tests to only use flat pointers is annoying and not a sustainable testing strategy.

@nunoplopes
Copy link
Member

Yep, only the default address space is supported ATM.
There's no ETA for implementing this, sorry.

@nunoplopes nunoplopes changed the title Alive should not immediately give up on non-0 address spaces Add support for non-0 address spaces Mar 20, 2023
@nunoplopes nunoplopes added enhancement New feature or request memory Memory Model labels Mar 20, 2023
@FlashSheridan
Copy link
Contributor

We needed non-0 address spaces, plus 256-bit pointers, so I started experimenting by simply changing the address space assertions to warnings, and raising the naked 64 limits. (Given the seriousness of misoptimization for financial applications, I was willing to tolerate a lot of false positives to find even a small number of bugs. Obviously I had to accept the possibility of false negatives for now.)
     I couldn’t get tv.dylib to work with our 17.0.6 fork, but the simple changes allowed some testing with the current alive-tv on one of our LLVM IR tests, splitting the output from --print-after-all with our fork, and testing the transitions. There were a manageable number of false positives, some of which were understandable; the apparently unjustified ones went away when I massaged the file with regexes to use only the default address space and 64-bit pointers. So far no more bugs found in our code.
     Testing on the entire -O3 optimization for the LLVM IR generated from a sample smart contract currently asserts "cast<Ty>() argument of incompatible type" in hasNoSignedWrap in llvm-project/llvm/include/llvm/IR/Instruction.h, called from llvm2alive.cpp:1753. My next step is to split up that optimization as well, and see whether any individual transition gives an unsoundness report which looks genuine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request memory Memory Model
Projects
None yet
Development

No branches or pull requests

3 participants