[PATCH] D82703: [InstCombine] convert assumes to operand bundles

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a subscriber: lebedev.ri.
jdoerfert added a comment.

Looks conceptually good but there are some oddities, potentially in the lookup.



================
Comment at: llvm/test/Transforms/InstCombine/assume.ll:56
+; BUNDLES-NEXT:    [[TMP0:%.*]] = load i32, i32* [[A]], align 4
+; BUNDLES-NEXT:    ret i32 [[TMP0]]
 ;
----------------
This doesn't seem to work properly.

@lebedev.ri found other problematic cases. One that I now know of is multiple bundles: https://godbolt.org/z/To5obz 


================
Comment at: llvm/test/Transforms/InstCombine/assume.ll:350
+; BUNDLES-NEXT:    [[RVAL_2:%.*]] = icmp sgt i32* [[LOAD]], null
+; BUNDLES-NEXT:    ret i1 [[RVAL_2]]
 ;
----------------
Can u add a FIXME or NOTE here to indicate we could duplicate the load instead and keep a `assume` with `["nonnull"(%load)]` around.


================
Comment at: llvm/test/Transforms/InstCombine/assume.ll:553
+; BUNDLES:       not_taken:
+; BUNDLES-NEXT:    ret i1 [[CONTROL]]
 ;
----------------
Again a fold missing `CMP = true`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82703