[PATCH] D155028: [ConstantHoisting] add XFAIL test case

Fri Aug 4 09:23:07 PDT 2023

nickdesaulniers added inline comments.

================
Comment at: llvm/test/Transforms/ConstantHoisting/X86/bad-cases.ll:130
+; The intent of this test is to verify that 68719476705 does not get
+; transformed into 68719476704 + 1 in basic block %d.
+define i1 @foo (ptr %bar) {
----------------
ributzka wrote:
> Why shouldn't this expensive constant be hoisted? The `68719476704` should be hoisted out of the BB and only the `add` should stay in %d. Why is  `68719476704` materialized twice?
> Why shouldn't this expensive constant be hoisted?

I'm not even sure it's expensive on x86_64. Pretty sure it can fit in an immediate field for stores + comparisons.

> The 68719476704 should be hoisted out of the BB and only the add should stay in %d.

Sure, that would have been acceptable, too.

> Why is 68719476704 materialized twice?

That's what I'm curious about; this seems like a bug in ConstantHoist, right?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155028