[PATCH] D139550: [DAGCombine] Fix always true condition in combineShiftToMULH

Juan Manuel Martinez CaamaƱo via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 12 05:12:42 PST 2022


jmmartinez added a comment.

In D139550#3978683 <https://reviews.llvm.org/D139550#3978683>, @RKSimon wrote:

> Looking at D133768 <https://reviews.llvm.org/D133768> it looks like you could add additional test case to dagcomb-mullohi.ll which has 2 'upper bit only' users (srl and sra)?

I'm not sure I follow. Isn't that case covered by the following function in the test case?

  define i32 @mul_one_bit_hi_hi_u32(i32 %arg, i32 %arg1, i32* %arg2) {
    bb:
    %i = zext i32 %arg to i64
    %i3 = zext i32 %arg1 to i64
    %i4 = mul nsw i64 %i3, %i
    %i5 = lshr i64 %i4, 32
    %i6 = trunc i64 %i5 to i32
    store i32 %i6, i32* %arg2, align 4
    %i7 = lshr i64 %i4, 33
    %i8 = trunc i64 %i7 to i32
    ret i32 %i8
  }

Or you mean having a srl and a sra in the same test-case?

  define i32 @foo(i32 %arg, i32 %arg1, i32* %arg2) {
    bb:
    %i = zext i32 %arg to i64
    %i3 = zext i32 %arg1 to i64
    %i4 = mul nsw i64 %i3, %i
    %i5 = lshr i64 %i4, 32
    %i6 = trunc i64 %i5 to i32
    store i32 %i6, i32* %arg2, align 4
    %i7 = ashr i64 %i4, 33                  <-- change from logic to arithmetic
    %i8 = trunc i64 %i7 to i32
    ret i32 %i8
  }

Sorry for the delay, I was out-of-office.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139550



More information about the llvm-commits mailing list