[PATCH] D140858: [InstCombine]: Don't simplify bits if it causes imm32 to become imm64

Noah Goldstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 5 12:20:18 PST 2023


goldstein.w.n added a comment.

In D140858#4028085 <https://reviews.llvm.org/D140858#4028085>, @nikic wrote:

> I'm fine with this change, but let's wait for @spatel. The zext->sext changes in induction.ll don't look particularly problematic to me.

I found another issue,
I changed the code to just disable `ShrinkDemandedConstant` and am notice a fair amount more 
optimizations are breaking. We are just getting "lucky" with this patch b.c the test coverage
doesn't have constants that cross type size boundaries.

>From what I can tell the following tests need to be fixed:

  llvm/test/Transforms/InstCombine/demand_shrink_nsw.ll::
  - foo
  
  llvm/test/Transforms/InstCombine/icmp-and-shift.ll:
  - icmp_eq_and_pow2_shl_pow2_negative1
  - icmp_eq_and_pow2_shl_pow2_negative3
  - icmp_eq_and_pow2_minus1_shl1
  - icmp_eq_and_pow2_minus1_shl1_vec
  - icmp_ne_and_pow2_minus1_shl1
  - icmp_ne_and_pow2_minus1_shl1_vec
  - icmp_eq_and_pow2_minus1_shl_pow2
  - icmp_eq_and_pow2_minus1_shl_pow2_vec
  - icmp_ne_and_pow2_minus1_shl_pow2
  
  llvm/test/Transforms/InstCombine/icmp-mul-and.ll:
  - mul_mask_pow2_sgt0
  - mul_mask_pow2_eq4
  - pr40493
  - pr40493_neg1 (Maybe)
  - pr51551_neg1
  
  llvm/test/Transforms/InstCombine/icmp.ll:
  - test17vec
  - test17a
  - test20vec
  - test20a
  
  llvm/test/Transforms/InstCombine/select-ctlz-to-cttz.ll:
  - PR45762
  - PR45762_logical
  
  llvm/test/Transforms/InstCombine/sub.ll:
  - demand_low_bits_uses_commute

And then a variety of cases where we will lose `cmp r, 0` which is often better than `cmp r, C`.

Alot of analysis/transforms seems to use the actual constant values rather than the known-needed constant values.

I think maybe adding a seperate pass that runs right before `llvm.assume` is lost will be a better way to do this.
Otherwise think we should fix all those cases first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140858



More information about the llvm-commits mailing list