After InstCombine pass, expected output got changed

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?

Thanks and Regards,
Rohit Aggarwal
Compiler Engineer - AMD

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.

sample.ll.tar.gz (48.2 KB)

Hi,

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)
  • 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.png

Thanks,
Rohit Aggarwal

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.

Hi Florian,

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.

Yes, this poison which is generated is handled differently in llvm-trunk and reverted commit code down the pipeline.

  • Now in the release branch, this poison value is propagated down to ‘or’ inst. and is constantfolds %or = or true poison → poison
  • where as in the reverted code, %or = or true poison → true

Here is the main reason, where the problem started

  • In the release branch, 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.

Reattaching the image. PFA for more clarity.

Thanks,
Rohit Aggarwal

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.

Sure, the change in instcombine propagates poison more aggressively. But as far as I can tell, this is in line with the semantics specified in LangRef and the transformation is correct. The reason for the change in output seems to be that instcombine was taught to exploit the undefined behavior in this case.

If this change in output is causing any issues, I’d suggest you check the source file from which the LLVM IR has been generated for any undefined behavior and fix it.

If the input source file is well defined, I’d suggest to check where the poison value causing issues comes from.

Cheers,
Florian

The highlighted transformations are all correct. If there’s a bug in LLVM it’s somewhere else. But it’s more likely it’s a bug in the source program.

You can use Alive2 to check correctness of optimizations, e.g.: https://alive2.llvm.org/ce/z/rbZbY7

Nuno