[PATCH] D112256: [FPEnv][EarlyCSE] Add support for CSE of constrained FP intrinsics, take 2

Mon Aug 1 06:26:18 PDT 2022

spatel added inline comments.

================
Comment at: llvm/test/Transforms/EarlyCSE/tfpropagation.ll:125

 define double @branching_maytrap(i64 %a) #0 {
 ; CHECK-LABEL: @branching_maytrap(
----------------
kpn wrote:
> spatel wrote:
> > Am I seeing this correctly - all tests now transform?
> > Should there be tests where one or both of the constrained ops is "fpexcept.strict"?
> > Please add some explanatory comments for these tests.
> I precommitted the tests. The tests that aren't showing up in the diff now were the negative tests that included "fpexcept.strict" and the dynamic rounding tests. Since those test for changes that must not happen, this patch here shows no changes to those tests.
> 
> What sort of explanatory comments do you have in mind?
I'm still not seeing it - are the tests with fpexcept.strict not in this file?
It's not clear to me what conditions are needed to propagate the true/false - is it the same as the other transforms (must not be fpexcept.strict and must not be dynamic rounding)?

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112256/new/

https://reviews.llvm.org/D112256