In this patch, we remove the int2ptr [ptr2int (p)] if p is dereferenceable.
If the user of the round-trip cast is dominated by a load/store instruction of the same address, then we
can replace round-trip cast with bitcast.
Details
- Reviewers
nlopes aqjune nikic spatel lebedev.ri
Diff Detail
- Repository
- rG LLVM Github Monorepo
Time | Test | |
---|---|---|
30 ms | x64 debian > LLVM.Transforms/InstCombine::load-int2ptr-fold.ll Script:
--
: 'RUN: at line 2'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/opt -instcombine -S -disable-i2p-p2i-opt < /var/lib/buildkite-agent/builds/llvm-project/llvm/test/Transforms/InstCombine/load-int2ptr-fold.ll | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/Transforms/InstCombine/load-int2ptr-fold.ll
| |
70 ms | x64 windows > LLVM.Transforms/InstCombine::load-int2ptr-fold.ll Script:
--
: 'RUN: at line 2'; c:\ws\w9\llvm-project\premerge-checks\build\bin\opt.exe -instcombine -S -disable-i2p-p2i-opt < C:\ws\w9\llvm-project\premerge-checks\llvm\test\Transforms\InstCombine\load-int2ptr-fold.ll | c:\ws\w9\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w9\llvm-project\premerge-checks\llvm\test\Transforms\InstCombine\load-int2ptr-fold.ll
|
Event Timeline
Hi,
I became to wonder what happens if a pointer is freed in the function.
For example:
// assume that %p is dereferenceable(8) call void @g(%p); %q = inttoptr (ptrtoint %p); use(%q); // can we replace this with %p?
I guess the answer is no, because if @g freed %p and allocated an object at %p's location, replacing inttoptr(ptrtoint %p) with %p makes the program more undefined.
Is the semantics of dereferenceable(8) %p implying %p must not be freed in the function as well?
If it isn't I think we should check whether nofree flag is attached to the functions (@test, @test1, ...) as well.
I agree with you, but this is a bug in isDereferenceableAndAlignedPointer. This API should receive the instruction where we want to prove the ptr is dereferenceable and take care of free, etc.
So I would not block this patch on this.
The code here looks very weird.
Please can you at least split this up into a preparatory NFC changes, the deref-based fold, and the domination-based fold?
Also, please precommit the tests, even if just locally for you, so that the patches show how the check lines change.
llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp | ||
---|---|---|
348 | Well, nofree is still not very common. Which would render this optimization useless. Plus it would be unnecessary (and restrictive) to check once isDereferenceableAndAlignedPointer is fixed. |
llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp | ||
---|---|---|
348 | Okay, then I'm fine with the code leaving as it is here. |