Skip to content

Commit

Permalink
[IR] Mark lshr and ashr constant expressions as undesirable
Browse files Browse the repository at this point in the history 
These will no longer be created by default during constant folding.
  • Loading branch information
@nikic
nikic committed Nov 10, 2023
1 parent cd7ba9f commit 82f68a9
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 41 deletions.
88 changes: 50 additions & 38 deletions clang/test/Analysis/builtin_signbit.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,60 +12,72 @@ long double ld = -1.0L;
// CHECK-BE32-LABEL: define dso_local void @_Z12test_signbitv(
// CHECK-BE32-SAME: ) #[[ATTR0:[0-9]+]] {
// CHECK-BE32-NEXT: entry:
// CHECK-BE32-NEXT: [[FROMBOOL:%.*]] = zext i1 icmp slt (i64 trunc (i128 lshr (i128 bitcast (ppc_fp128 0xM3FF00000000000000000000000000000 to i128), i128 64) to i64), i64 0) to i8
// CHECK-BE32-NEXT: [[TMP0:%.*]] = lshr i128 bitcast (ppc_fp128 0xM3FF00000000000000000000000000000 to i128), 64
// CHECK-BE32-NEXT: [[TMP1:%.*]] = trunc i128 [[TMP0]] to i64
// CHECK-BE32-NEXT: [[TMP2:%.*]] = icmp slt i64 [[TMP1]], 0
// CHECK-BE32-NEXT: [[FROMBOOL:%.*]] = zext i1 [[TMP2]] to i8
// CHECK-BE32-NEXT: store i8 [[FROMBOOL]], ptr @b, align 1
// CHECK-BE32-NEXT: [[TMP0:%.*]] = load ppc_fp128, ptr @ld, align 16
// CHECK-BE32-NEXT: [[TMP1:%.*]] = bitcast ppc_fp128 [[TMP0]] to i128
// CHECK-BE32-NEXT: [[TMP2:%.*]] = lshr i128 [[TMP1]], 64
// CHECK-BE32-NEXT: [[TMP3:%.*]] = trunc i128 [[TMP2]] to i64
// CHECK-BE32-NEXT: [[TMP4:%.*]] = icmp slt i64 [[TMP3]], 0
// CHECK-BE32-NEXT: [[FROMBOOL1:%.*]] = zext i1 [[TMP4]] to i8
// CHECK-BE32-NEXT: [[TMP3:%.*]] = load ppc_fp128, ptr @ld, align 16
// CHECK-BE32-NEXT: [[TMP4:%.*]] = bitcast ppc_fp128 [[TMP3]] to i128
// CHECK-BE32-NEXT: [[TMP5:%.*]] = lshr i128 [[TMP4]], 64
// CHECK-BE32-NEXT: [[TMP6:%.*]] = trunc i128 [[TMP5]] to i64
// CHECK-BE32-NEXT: [[TMP7:%.*]] = icmp slt i64 [[TMP6]], 0
// CHECK-BE32-NEXT: [[FROMBOOL1:%.*]] = zext i1 [[TMP7]] to i8
// CHECK-BE32-NEXT: store i8 [[FROMBOOL1]], ptr @b, align 1
// CHECK-BE32-NEXT: store i8 0, ptr @b, align 1
// CHECK-BE32-NEXT: [[TMP5:%.*]] = load double, ptr @d, align 8
// CHECK-BE32-NEXT: [[CONV:%.*]] = fptrunc double [[TMP5]] to float
// CHECK-BE32-NEXT: [[TMP6:%.*]] = bitcast float [[CONV]] to i32
// CHECK-BE32-NEXT: [[TMP7:%.*]] = icmp slt i32 [[TMP6]], 0
// CHECK-BE32-NEXT: [[FROMBOOL2:%.*]] = zext i1 [[TMP7]] to i8
// CHECK-BE32-NEXT: [[TMP8:%.*]] = load double, ptr @d, align 8
// CHECK-BE32-NEXT: [[CONV:%.*]] = fptrunc double [[TMP8]] to float
// CHECK-BE32-NEXT: [[TMP9:%.*]] = bitcast float [[CONV]] to i32
// CHECK-BE32-NEXT: [[TMP10:%.*]] = icmp slt i32 [[TMP9]], 0
// CHECK-BE32-NEXT: [[FROMBOOL2:%.*]] = zext i1 [[TMP10]] to i8
// CHECK-BE32-NEXT: store i8 [[FROMBOOL2]], ptr @b, align 1
// CHECK-BE32-NEXT: [[FROMBOOL3:%.*]] = zext i1 icmp slt (i64 trunc (i128 lshr (i128 bitcast (ppc_fp128 0xM3FF00000000000000000000000000000 to i128), i128 64) to i64), i64 0) to i8
// CHECK-BE32-NEXT: [[TMP11:%.*]] = lshr i128 bitcast (ppc_fp128 0xM3FF00000000000000000000000000000 to i128), 64
// CHECK-BE32-NEXT: [[TMP12:%.*]] = trunc i128 [[TMP11]] to i64
// CHECK-BE32-NEXT: [[TMP13:%.*]] = icmp slt i64 [[TMP12]], 0
// CHECK-BE32-NEXT: [[FROMBOOL3:%.*]] = zext i1 [[TMP13]] to i8
// CHECK-BE32-NEXT: store i8 [[FROMBOOL3]], ptr @b, align 1
// CHECK-BE32-NEXT: [[TMP8:%.*]] = load ppc_fp128, ptr @ld, align 16
// CHECK-BE32-NEXT: [[TMP9:%.*]] = bitcast ppc_fp128 [[TMP8]] to i128
// CHECK-BE32-NEXT: [[TMP10:%.*]] = lshr i128 [[TMP9]], 64
// CHECK-BE32-NEXT: [[TMP11:%.*]] = trunc i128 [[TMP10]] to i64
// CHECK-BE32-NEXT: [[TMP12:%.*]] = icmp slt i64 [[TMP11]], 0
// CHECK-BE32-NEXT: [[FROMBOOL4:%.*]] = zext i1 [[TMP12]] to i8
// CHECK-BE32-NEXT: [[TMP14:%.*]] = load ppc_fp128, ptr @ld, align 16
// CHECK-BE32-NEXT: [[TMP15:%.*]] = bitcast ppc_fp128 [[TMP14]] to i128
// CHECK-BE32-NEXT: [[TMP16:%.*]] = lshr i128 [[TMP15]], 64
// CHECK-BE32-NEXT: [[TMP17:%.*]] = trunc i128 [[TMP16]] to i64
// CHECK-BE32-NEXT: [[TMP18:%.*]] = icmp slt i64 [[TMP17]], 0
// CHECK-BE32-NEXT: [[FROMBOOL4:%.*]] = zext i1 [[TMP18]] to i8
// CHECK-BE32-NEXT: store i8 [[FROMBOOL4]], ptr @b, align 1
// CHECK-BE32-NEXT: ret void
//
// CHECK-BE64-LABEL: define dso_local void @_Z12test_signbitv(
// CHECK-BE64-SAME: ) #[[ATTR0:[0-9]+]] {
// CHECK-BE64-NEXT: entry:
// CHECK-BE64-NEXT: [[FROMBOOL:%.*]] = zext i1 icmp slt (i64 trunc (i128 lshr (i128 bitcast (ppc_fp128 0xM3FF00000000000000000000000000000 to i128), i128 64) to i64), i64 0) to i8
// CHECK-BE64-NEXT: [[TMP0:%.*]] = lshr i128 bitcast (ppc_fp128 0xM3FF00000000000000000000000000000 to i128), 64
// CHECK-BE64-NEXT: [[TMP1:%.*]] = trunc i128 [[TMP0]] to i64
// CHECK-BE64-NEXT: [[TMP2:%.*]] = icmp slt i64 [[TMP1]], 0
// CHECK-BE64-NEXT: [[FROMBOOL:%.*]] = zext i1 [[TMP2]] to i8
// CHECK-BE64-NEXT: store i8 [[FROMBOOL]], ptr @b, align 1
// CHECK-BE64-NEXT: [[TMP0:%.*]] = load ppc_fp128, ptr @ld, align 16
// CHECK-BE64-NEXT: [[TMP1:%.*]] = bitcast ppc_fp128 [[TMP0]] to i128
// CHECK-BE64-NEXT: [[TMP2:%.*]] = lshr i128 [[TMP1]], 64
// CHECK-BE64-NEXT: [[TMP3:%.*]] = trunc i128 [[TMP2]] to i64
// CHECK-BE64-NEXT: [[TMP4:%.*]] = icmp slt i64 [[TMP3]], 0
// CHECK-BE64-NEXT: [[FROMBOOL1:%.*]] = zext i1 [[TMP4]] to i8
// CHECK-BE64-NEXT: [[TMP3:%.*]] = load ppc_fp128, ptr @ld, align 16
// CHECK-BE64-NEXT: [[TMP4:%.*]] = bitcast ppc_fp128 [[TMP3]] to i128
// CHECK-BE64-NEXT: [[TMP5:%.*]] = lshr i128 [[TMP4]], 64
// CHECK-BE64-NEXT: [[TMP6:%.*]] = trunc i128 [[TMP5]] to i64
// CHECK-BE64-NEXT: [[TMP7:%.*]] = icmp slt i64 [[TMP6]], 0
// CHECK-BE64-NEXT: [[FROMBOOL1:%.*]] = zext i1 [[TMP7]] to i8
// CHECK-BE64-NEXT: store i8 [[FROMBOOL1]], ptr @b, align 1
// CHECK-BE64-NEXT: store i8 0, ptr @b, align 1
// CHECK-BE64-NEXT: [[TMP5:%.*]] = load double, ptr @d, align 8
// CHECK-BE64-NEXT: [[CONV:%.*]] = fptrunc double [[TMP5]] to float
// CHECK-BE64-NEXT: [[TMP6:%.*]] = bitcast float [[CONV]] to i32
// CHECK-BE64-NEXT: [[TMP7:%.*]] = icmp slt i32 [[TMP6]], 0
// CHECK-BE64-NEXT: [[FROMBOOL2:%.*]] = zext i1 [[TMP7]] to i8
// CHECK-BE64-NEXT: [[TMP8:%.*]] = load double, ptr @d, align 8
// CHECK-BE64-NEXT: [[CONV:%.*]] = fptrunc double [[TMP8]] to float
// CHECK-BE64-NEXT: [[TMP9:%.*]] = bitcast float [[CONV]] to i32
// CHECK-BE64-NEXT: [[TMP10:%.*]] = icmp slt i32 [[TMP9]], 0
// CHECK-BE64-NEXT: [[FROMBOOL2:%.*]] = zext i1 [[TMP10]] to i8
// CHECK-BE64-NEXT: store i8 [[FROMBOOL2]], ptr @b, align 1
// CHECK-BE64-NEXT: [[FROMBOOL3:%.*]] = zext i1 icmp slt (i64 trunc (i128 lshr (i128 bitcast (ppc_fp128 0xM3FF00000000000000000000000000000 to i128), i128 64) to i64), i64 0) to i8
// CHECK-BE64-NEXT: [[TMP11:%.*]] = lshr i128 bitcast (ppc_fp128 0xM3FF00000000000000000000000000000 to i128), 64
// CHECK-BE64-NEXT: [[TMP12:%.*]] = trunc i128 [[TMP11]] to i64
// CHECK-BE64-NEXT: [[TMP13:%.*]] = icmp slt i64 [[TMP12]], 0
// CHECK-BE64-NEXT: [[FROMBOOL3:%.*]] = zext i1 [[TMP13]] to i8
// CHECK-BE64-NEXT: store i8 [[FROMBOOL3]], ptr @b, align 1
// CHECK-BE64-NEXT: [[TMP8:%.*]] = load ppc_fp128, ptr @ld, align 16
// CHECK-BE64-NEXT: [[TMP9:%.*]] = bitcast ppc_fp128 [[TMP8]] to i128
// CHECK-BE64-NEXT: [[TMP10:%.*]] = lshr i128 [[TMP9]], 64
// CHECK-BE64-NEXT: [[TMP11:%.*]] = trunc i128 [[TMP10]] to i64
// CHECK-BE64-NEXT: [[TMP12:%.*]] = icmp slt i64 [[TMP11]], 0
// CHECK-BE64-NEXT: [[FROMBOOL4:%.*]] = zext i1 [[TMP12]] to i8
// CHECK-BE64-NEXT: [[TMP14:%.*]] = load ppc_fp128, ptr @ld, align 16
// CHECK-BE64-NEXT: [[TMP15:%.*]] = bitcast ppc_fp128 [[TMP14]] to i128
// CHECK-BE64-NEXT: [[TMP16:%.*]] = lshr i128 [[TMP15]], 64
// CHECK-BE64-NEXT: [[TMP17:%.*]] = trunc i128 [[TMP16]] to i64
// CHECK-BE64-NEXT: [[TMP18:%.*]] = icmp slt i64 [[TMP17]], 0
// CHECK-BE64-NEXT: [[FROMBOOL4:%.*]] = zext i1 [[TMP18]] to i8
// CHECK-BE64-NEXT: store i8 [[FROMBOOL4]], ptr @b, align 1
// CHECK-BE64-NEXT: ret void
//
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/IR/Constants.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2133,13 +2133,13 @@ bool ConstantExpr::isDesirableBinOp(unsigned Opcode) {
case Instruction::FRem:
case Instruction::And:
case Instruction::Or:
case Instruction::LShr:
case Instruction::AShr:
return false;
case Instruction::Add:
case Instruction::Sub:
case Instruction::Mul:
case Instruction::Shl:
case Instruction::LShr:
case Instruction::AShr:
case Instruction::Xor:
return true;
default:
Expand Down
3 changes: 2 additions & 1 deletion llvm/test/CodeGen/AArch64/stack-tagging-initializer-merge.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,8 @@ entry:

; CHECK-LABEL: define void @InitVectorSplit(
; CHECK: [[TX:%.*]] = call ptr @llvm.aarch64.tagp
; CHECK: call void @llvm.aarch64.stgp(ptr [[TX]], i64 shl (i64 bitcast (<2 x i32> <i32 1, i32 2> to i64), i64 32), i64 lshr (i64 bitcast (<2 x i32> <i32 1, i32 2> to i64), i64 32))
; CHECK: [[LSHR:%.*]] = lshr i64 bitcast (<2 x i32> <i32 1, i32 2> to i64), 32
; CHECK: call void @llvm.aarch64.stgp(ptr [[TX]], i64 shl (i64 bitcast (<2 x i32> <i32 1, i32 2> to i64), i64 32), i64 [[LSHR]])
; CHECK: ret void

define void @MemSetZero() sanitize_memtag {
Expand Down

4 comments on commit 82f68a9

@smeenai
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This depended on cd7ba9f, and that was causing test failures in no-asserts builds, so I reverted both.

@nathanchance
Copy link
Member

@nathanchance nathanchance commented on 82f68a9 Nov 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change causes an infinite loop when building the Linux kernel. An LLVM IR reproducer from llvm-reduce:

target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-pc-linux-gnu"

@empty_zero_page = external global [0 x i64]

define void @__attribute__init_zero_pfn(ptr %zero_pfn) {
entry:
  %shr = ashr i64 ptrtoint (ptr @empty_zero_page to i64), 1
  %sub.ptr.div = sdiv i64 %shr, 4
  store i64 %sub.ptr.div, ptr %zero_pfn, align 8
  ret void
}

At cd7ba9f:

$ hyperfine -N 'opt -O2 -disable-output reduced.ll'
Benchmark 1: opt -O2 -disable-output reduced.ll
  Time (mean ± σ):       8.4 ms ±   0.2 ms    [User: 2.6 ms, System: 5.7 ms]
  Range (min … max):     7.3 ms …   9.2 ms    339 runs

At this change:

$ timeout 10s opt -O2 -disable-output reduced.ll

$ echo $?
124
Bisect log 
# bad: [18415c8365047841c4671798e0129ca9bbd03c40] [DebugInfo] Add debug info test for jump table (#71018)
# good: [f175b9647ccdfd67300264b2d3bd76e6f9a3fb93] Improve VSCode DAP logpoint value summary (#71723)
git bisect start '18415c8365047841c4671798e0129ca9bbd03c40' 'f175b9647ccdfd67300264b2d3bd76e6f9a3fb93'
# good: [7c3603e1c162382b5c038b99e482e0689e1505aa] [lldb][test] Only add -m(64|32) for GCC on non Arm/AArch64 platforms
git bisect good 7c3603e1c162382b5c038b99e482e0689e1505aa
# bad: [a93dfb589d17245aa43d76df8e0835b019e04b7a] [RISCV] Peek through zext in selectShiftMask.
git bisect bad a93dfb589d17245aa43d76df8e0835b019e04b7a
# bad: [cf1e3420b0141d84770869205cfdb546609014dd] [flang][windows] Add option to link against specific MSVC CRT (#70833)
git bisect bad cf1e3420b0141d84770869205cfdb546609014dd
# good: [b008d66cd33b88ca1a49c214ddb6d4a52523d2d7] [Clang][SME2] Add single and multi min and max builtins (#71688)
git bisect good b008d66cd33b88ca1a49c214ddb6d4a52523d2d7
# bad: [af8ebfdcd9d633572ea25eb12407eb92c18e563a] [NVPTX] Allow the ctor/dtor lowering pass to emit kernels (#71549)
git bisect bad af8ebfdcd9d633572ea25eb12407eb92c18e563a
# good: [d4d289144764f4d293874d8959cc0d65ff79148f] [mlir][vector] Add distribution pattern for vector.create_mask (#71619)
git bisect good d4d289144764f4d293874d8959cc0d65ff79148f
# good: [cd7ba9f3d090afb5d3b15b0dcf379d15d1e11e33] [Clang] Generate test checks (NFC)
git bisect good cd7ba9f3d090afb5d3b15b0dcf379d15d1e11e33
# bad: [3a9cc17ca088267348e4b4a6e64a88a38ae9c6e4] [Clang] Add missing REQUIRES to tests (NFC)
git bisect bad 3a9cc17ca088267348e4b4a6e64a88a38ae9c6e4
# bad: [82f68a992b9f89036042d57a5f6345cb2925b2c1] [IR] Mark lshr and ashr constant expressions as undesirable
git bisect bad 82f68a992b9f89036042d57a5f6345cb2925b2c1
# first bad commit: [82f68a992b9f89036042d57a5f6345cb2925b2c1] [IR] Mark lshr and ashr constant expressions as undesirable

@nikic
Copy link
Contributor Author

@nikic nikic commented on 82f68a9 Nov 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nathanchance Thanks for the report! I believe that 002da67 should fix this issue.

@nathanchance
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed, at least for this one case :) I'll do more builds and follow up with separate reports if I run into anything else. Thanks for the quick fix!

Please sign in to comment.