Rohit Aggarwal via llvm-dev
2021-Aug-19 10:00 UTC
[llvm-dev] After InstCombine pass, expected output got changed
Hi All, Thanks Florian for response. Adding more details to the issue: - I revert the commit [ConstantFold] Allow propagation of poison for and/or i1 <https://github.com/llvm/llvm-project/commit/2fd3037ac615643fe8058292d2b89bb19a49cb2f#> then it is working fine. - In this commit, a check is removed to stop the constantfold of instruction to poison value incase of integer ADD or OR instruction if either of operands are poison. - Using both release branch and commit reverted branch, a poison value is generated by a shift left instruction due to overflow (UB is present) - Now in the release branch, this poison value is propagated down to 'or' inst. and is constantfolds %or = or true poison -> poison - The poison value propagates to its uses - There is a store instruction which now has a poison value - In visitStoreInst, store instruction got removed due to poison value - But in revert branch, store is not getting remove as it has defined value See the below diff for more clarity. [image: image.png] Thanks, Rohit Aggarwal On Thu, Aug 19, 2021 at 2:19 PM Florian Hahn <florian_hahn at apple.com> wrote:> Hi, > > On Aug 18, 2021, at 21:21, Rohit Aggarwal via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > > > Hi All, > > I am running the instcombine pass on the given lR and executing the binary. > > *./build_release13x/bin/opt -enable-new-pm=false -instcombine sample.ll > -S -o sample_instcombined_llvm.ll && ./build_release13x/bin/clang > sample_instcombined_llvm.ll && ./a.out* > > But before passing to instcombine, the output is *CE6DB4AC* and after > passing to instcombine value changes to *3B83FBC*. > > I tried to debug the issue and found that due to the occurence of poison > value, some instructions(or, store,..) are getting deleted/erased. > > I also observed that there are active changes happening related to poison > values in the community. Is this behaviour expected one? Or Could it be a > side effect of those changes > > > There are 2 potential causes for the difference: 1) your input has > undefined behavior and/or the result depends on a poison value or 2) a bug > in instcombine. > > Without narrowing things down to the problematic pattern, it is very hard > to say which one it is. I’d suggest you try to isolate the change in > behavior to the problematic instcombine pattern and minimize the input IR > to trigger the problematic pattern. > > Cheers, > Florian >-- Disclaimer:- This footer text is to convey that this email is sent by one of the users of IITH. So, do not mark it as SPAM. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210819/7ce8b2ae/attachment.html>
Florian Hahn via llvm-dev
2021-Aug-19 10:16 UTC
[llvm-dev] After InstCombine pass, expected output got changed
> On 19 Aug 2021, at 11:01, Rohit Aggarwal via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > > Hi All, > > Thanks Florian for response. > > Adding more details to the issue: > I revert the commit [ConstantFold] Allow propagation of poison for and/or i1 then it is working fine. > In this commit, a check is removed to stop the constantfold of instruction to poison value incase of integer ADD or OR instruction if either of operands are poison. > Using both release branch and commit reverted branch, a poison value is generated by a shift left instruction due to overflow (UB is present)Ok so it sounds like this is the reason for the different output and the transformation is correct.> Now in the release branch, this poison value is propagated down to 'or' inst. and is constantfolds %or = or true poison -> poison > The poison value propagates to its uses > There is a store instruction which now has a poison value > In visitStoreInst, store instruction got removed due to poison value > But in revert branch, store is not getting remove as it has defined value > See the below diff for more clarity. > > > Thanks, > Rohit Aggarwal > >> On Thu, Aug 19, 2021 at 2:19 PM Florian Hahn <florian_hahn at apple.com> wrote: >> Hi, >> >>> On Aug 18, 2021, at 21:21, Rohit Aggarwal via llvm-dev <llvm-dev at lists.llvm.org> wrote: >>> >>> >>> Hi All, >>> >>> I am running the instcombine pass on the given lR and executing the binary. >>> >>> ./build_release13x/bin/opt -enable-new-pm=false -instcombine sample.ll -S -o sample_instcombined_llvm.ll && ./build_release13x/bin/clang sample_instcombined_llvm.ll && ./a.out >>> >>> But before passing to instcombine, the output is CE6DB4AC and after passing to instcombine value changes to 3B83FBC. >>> >>> I tried to debug the issue and found that due to the occurence of poison value, some instructions(or, store,..) are getting deleted/erased. >>> >>> I also observed that there are active changes happening related to poison values in the community. Is this behaviour expected one? Or Could it be a side effect of those changes >> >> There are 2 potential causes for the difference: 1) your input has undefined behavior and/or the result depends on a poison value or 2) a bug in instcombine. >> >> Without narrowing things down to the problematic pattern, it is very hard to say which one it is. I’d suggest you try to isolate the change in behavior to the problematic instcombine pattern and minimize the input IR to trigger the problematic pattern. >> >> Cheers, >> Florian > > Disclaimer:- This footer text is to convey that this email is sent by one of the users of IITH. So, do not mark it as SPAM. > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210819/6db8b5cc/attachment.html>