[InstCombine] Merge foldFreezeIntoRecurrence into pushFreezeToPreventPoisonFromPropagating#171435
[InstCombine] Merge foldFreezeIntoRecurrence into pushFreezeToPreventPoisonFromPropagating#171435c-rhodes wants to merge 5 commits into
Conversation
|
@zyw-bot csmith-fuzz |
You can test this locally with the following command:git diff -U0 --pickaxe-regex -S '([^a-zA-Z0-9#_-]undef([^a-zA-Z0-9_-]|$)|UndefValue::get)' 'HEAD~1' HEAD llvm/lib/Transforms/InstCombine/InstructionCombining.cpp llvm/test/Transforms/InstCombine/freeze.llThe following files introduce new uses of undef:
Undef is now deprecated and should only be used in the rare cases where no replacement is possible. For example, a load of uninitialized memory yields In tests, avoid using For example, this is considered a bad practice: define void @fn() {
...
br i1 undef, ...
}Please use the following instead: define void @fn(i1 %cond) {
...
br i1 %cond, ...
}Please refer to the Undefined Behavior Manual for more information. |
Looking at this code again the Use/User distinction is a bit confusing, but I'm pretty sure if we bail when CanPushFreeze returns false for the PHI it should bring the behaviour inline with what was happening when this code was part of foldFreezeIntoRecurrence. Change was causing compile-time issues: dtcxzyw/llvm-opt-benchmark#3129 The smallest being: https://github.com/dtcxzyw/llvm-opt-benchmark/blob/main/bench/ffmpeg/original/avc.ll which I reduced to: https://godbolt.org/z/6Gx58GfK1
|
in the first iteration there were some hangs dtcxzyw/llvm-opt-benchmark#3129, I got a reduced testcase for acv and fixed it with the latest commit. Re-ran yesterday and there's more hangs: dtcxzyw/llvm-opt-benchmark#3130. I reduced miniz to: https://godbolt.org/z/cTqcxP9j4 it gets stuck in a loop of: which is caused by llvm-project/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp Lines 1581 to 1595 in 54b4bd5 adding nneg then pushFreezeToPreventPoisonFromPropagating dropping it.
|
Loop in https://godbolt.org/z/cTqcxP9j4 caused by https://github.com/llvm/llvm-project/blob/54b4bd510a8cc90fac3d2071179d1424af894c9c/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp#L1581-L1595 adding nneg then pushFreezeToPreventPoisonFromPropagating dropping it.
|
@c-rhodes I think it would great to commit tests for the infinite loop cases you reduced. That would help if this work gets picked up again in the future. |
good idea, will do |
Adds a number of test cases where we have to be careful when pushing freeze around. These first two tests are taken from llvm#157678 which failed to land to due compile-time issues. The last two tests were reduced from llvm-opt-benchmark workloads on llvm#171435 which was an attempt at addressing underlying cause of the hangs.
Adds a number of test cases where we have to be careful when pushing freeze around. These first two tests are taken from #157678 which failed to land to due compile-time issues. The last two tests were reduced from llvm-opt-benchmark workloads on #171435 which was an attempt at addressing underlying cause of the hangs.
Adds a number of test cases where we have to be careful when pushing freeze around. These first two tests are taken from llvm#157678 which failed to land to due compile-time issues. The last two tests were reduced from llvm-opt-benchmark workloads on llvm#171435 which was an attempt at addressing underlying cause of the hangs.
Adds a number of test cases where we have to be careful when pushing freeze around. These first two tests are taken from llvm#157678 which failed to land to due compile-time issues. The last two tests were reduced from llvm-opt-benchmark workloads on llvm#171435 which was an attempt at addressing underlying cause of the hangs.
No description provided.