|
@@ -0,0 +1,231 @@
|
|
|
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
|
+From: Antonio Maiorano <[email protected]>
|
|
|
+Date: Thu, 20 Jun 2024 15:39:00 -0400
|
|
|
+Subject: Fix instcombine overflow check inserting inst at wrong place (#6679)
|
|
|
+
|
|
|
+When optimizing an overflow check of an add followed by a compare, the
|
|
|
+new instruction was being inserted at the compare, and the add removed.
|
|
|
+This produced invalid IR in cases where there were other uses of the
|
|
|
+former add between it and the compare. This fix makes sure to insert the
|
|
|
+new instruction at the old add location, rather than at the compare.
|
|
|
+
|
|
|
+Note that this was also fixed in LLVM:
|
|
|
+
|
|
|
+https://github.com/llvm/llvm-project/commit/6f5dca70ed1c030957a45ad91bd295921f17b18d
|
|
|
+
|
|
|
+Bug: chromium:342545100
|
|
|
+Change-Id: Iecf758e4465b32371266bbe9879790328f363322
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/external/github.com/microsoft/DirectXShaderCompiler/+/5644335
|
|
|
+Reviewed-by: dan sinclair <[email protected]>
|
|
|
+Reviewed-by: Ryan Harrison <[email protected]>
|
|
|
+
|
|
|
+diff --git a/lib/Transforms/InstCombine/InstCombineCompares.cpp b/lib/Transforms/InstCombine/InstCombineCompares.cpp
|
|
|
+index 7274304b3de7b110a4fa3a5123ea7215d99935ef..82e7e49003e65846c84e3df2801453fb463aa23e 100644
|
|
|
+--- a/lib/Transforms/InstCombine/InstCombineCompares.cpp
|
|
|
++++ b/lib/Transforms/InstCombine/InstCombineCompares.cpp
|
|
|
+@@ -2145,6 +2145,12 @@ bool InstCombiner::OptimizeOverflowCheck(OverflowCheckFlavor OCF, Value *LHS,
|
|
|
+ return true;
|
|
|
+ };
|
|
|
+
|
|
|
++ // If the overflow check was an add followed by a compare, the insertion point
|
|
|
++ // may be pointing to the compare. We want to insert the new instructions
|
|
|
++ // before the add in case there are uses of the add between the add and the
|
|
|
++ // compare.
|
|
|
++ Builder->SetInsertPoint(&OrigI);
|
|
|
++
|
|
|
+ switch (OCF) {
|
|
|
+ case OCF_INVALID:
|
|
|
+ llvm_unreachable("bad overflow check kind!");
|
|
|
+diff --git a/tools/clang/test/DXC/Passes/InstructionCombining/instcombine-opt-overflow-check-inserts-at-add.ll b/tools/clang/test/DXC/Passes/InstructionCombining/instcombine-opt-overflow-check-inserts-at-add.ll
|
|
|
+new file mode 100644
|
|
|
+index 0000000000000000000000000000000000000000..70efa00c91c6523867980de0fb9f69c7fcfb318c
|
|
|
+--- /dev/null
|
|
|
++++ b/tools/clang/test/DXC/Passes/InstructionCombining/instcombine-opt-overflow-check-inserts-at-add.ll
|
|
|
+@@ -0,0 +1,187 @@
|
|
|
++; RUN: %dxopt %s -hlsl-passes-resume -instcombine,NoSink=0 -S | FileCheck %s
|
|
|
++
|
|
|
++; Generated from the following HLSL:
|
|
|
++; cbuffer cbuffer_g : register(b0) {
|
|
|
++; uint4 g[1];
|
|
|
++; };
|
|
|
++;
|
|
|
++; [numthreads(1, 1, 1)]
|
|
|
++; void main() {
|
|
|
++; uint a = 2147483648u;
|
|
|
++; uint b = (g[0].x | 2651317025u);
|
|
|
++; uint c = (b + 2651317025u);
|
|
|
++; while (true) {
|
|
|
++; bool d = (a > c);
|
|
|
++; if (d) {
|
|
|
++; break;
|
|
|
++; } else {
|
|
|
++; while (true) {
|
|
|
++; if (!d) {
|
|
|
++; return;
|
|
|
++; }
|
|
|
++; a = b;
|
|
|
++; bool e = (d ? d : d);
|
|
|
++; if (e) {
|
|
|
++; break;
|
|
|
++; }
|
|
|
++; }
|
|
|
++; }
|
|
|
++; }
|
|
|
++; }
|
|
|
++;
|
|
|
++; Compiling this was resulting in invalid IR being produced from instcombine.
|
|
|
++; Specifically, when optimizing an overflow check of an add followed by a compare,
|
|
|
++; the new instruction was being inserted at the compare, and the add removed. This
|
|
|
++; broke in cases like this one, where there were other uses of the former add between
|
|
|
++; it and the compare. The fix was to make sure to insert the new instruction
|
|
|
++; (another add in this case) at the old add rather than at the compare.
|
|
|
++
|
|
|
++; Make sure the new %add is still in the entry block before its uses.
|
|
|
++; CHECK-LABEL: entry
|
|
|
++; CHECK: %add = add i32 %or, -1643650271
|
|
|
++; CHECK-NEXT: %cmp.2 = icmp sgt i32 %add, -1,
|
|
|
++;
|
|
|
++; Make sure the new %add is NOT in the loopexit where %cmp was optimized.
|
|
|
++; CHECK-LABEL: while.body.loopexit
|
|
|
++; CHECK-NEXT: br i1 false, label %if.end.preheader, label %while.end.14
|
|
|
++
|
|
|
++;
|
|
|
++; Buffer Definitions:
|
|
|
++;
|
|
|
++; cbuffer cbuffer_g
|
|
|
++; {
|
|
|
++;
|
|
|
++; struct cbuffer_g
|
|
|
++; {
|
|
|
++;
|
|
|
++; uint4 g[1]; ; Offset: 0
|
|
|
++;
|
|
|
++; } cbuffer_g; ; Offset: 0 Size: 16
|
|
|
++;
|
|
|
++; }
|
|
|
++;
|
|
|
++;
|
|
|
++; Resource Bindings:
|
|
|
++;
|
|
|
++; Name Type Format Dim ID HLSL Bind Count
|
|
|
++; ------------------------------ ---------- ------- ----------- ------- -------------- ------
|
|
|
++; cbuffer_g cbuffer NA NA CB0 cb0 1
|
|
|
++;
|
|
|
++target datalayout = "e-m:e-p:32:32-i1:32-i8:32-i16:32-i32:32-i64:64-f16:32-f32:32-f64:64-n8:16:32:64"
|
|
|
++target triple = "dxil-ms-dx"
|
|
|
++
|
|
|
++%cbuffer_g = type { [1 x <4 x i32>] }
|
|
|
++%dx.types.Handle = type { i8* }
|
|
|
++%dx.types.ResourceProperties = type { i32, i32 }
|
|
|
++%dx.types.CBufRet.i32 = type { i32, i32, i32, i32 }
|
|
|
++
|
|
|
++@cbuffer_g = external constant %cbuffer_g
|
|
|
[email protected] = appending global [1 x i8*] [i8* bitcast (%cbuffer_g* @cbuffer_g to i8*)], section "llvm.metadata"
|
|
|
++
|
|
|
++; Function Attrs: nounwind
|
|
|
++define void @main() #0 {
|
|
|
++entry:
|
|
|
++ %0 = load %cbuffer_g, %cbuffer_g* @cbuffer_g
|
|
|
++ %cbuffer_g = call %dx.types.Handle @dx.op.createHandleForLib.cbuffer_g(i32 160, %cbuffer_g %0) ; CreateHandleForLib(Resource)
|
|
|
++ %1 = call %dx.types.Handle @dx.op.annotateHandle(i32 216, %dx.types.Handle %cbuffer_g, %dx.types.ResourceProperties { i32 13, i32 16 }) ; AnnotateHandle(res,props) resource: CBuffer
|
|
|
++ %2 = call %dx.types.CBufRet.i32 @dx.op.cbufferLoadLegacy.i32(i32 59, %dx.types.Handle %1, i32 0), !dbg !19 ; line:8 col:13 ; CBufferLoadLegacy(handle,regIndex)
|
|
|
++ %3 = extractvalue %dx.types.CBufRet.i32 %2, 0, !dbg !19 ; line:8 col:13
|
|
|
++ %or = or i32 %3, -1643650271, !dbg !23 ; line:8 col:20
|
|
|
++ %add = add i32 %or, -1643650271, !dbg !24 ; line:9 col:15
|
|
|
++ %cmp.2 = icmp ugt i32 -2147483648, %add, !dbg !25 ; line:11 col:17
|
|
|
++ %frombool.3 = zext i1 %cmp.2 to i32, !dbg !26 ; line:11 col:10
|
|
|
++ %tobool1.4 = icmp eq i32 %frombool.3, 0, !dbg !27 ; line:12 col:9
|
|
|
++ %or.cond.6 = and i1 %tobool1.4, %cmp.2, !dbg !27 ; line:12 col:9
|
|
|
++ br i1 %or.cond.6, label %if.end.preheader, label %while.end.14, !dbg !27 ; line:12 col:9
|
|
|
++
|
|
|
++while.body.loopexit: ; preds = %if.end
|
|
|
++ %cmp = icmp ugt i32 %or, %add, !dbg !25 ; line:11 col:17
|
|
|
++ %frombool = zext i1 %cmp to i32, !dbg !26 ; line:11 col:10
|
|
|
++ %tobool1 = icmp eq i32 %frombool, 0, !dbg !27 ; line:12 col:9
|
|
|
++ %or.cond = and i1 %tobool1, %cmp, !dbg !27 ; line:12 col:9
|
|
|
++ br i1 %or.cond, label %if.end.preheader, label %while.end.14, !dbg !27 ; line:12 col:9
|
|
|
++
|
|
|
++if.end.preheader: ; preds = %entry, %while.body.loopexit
|
|
|
++ %d.0 = phi i32 [ %frombool, %while.body.loopexit ], [ %frombool.3, %entry ]
|
|
|
++ br label %if.end, !dbg !28 ; line:19 col:13
|
|
|
++
|
|
|
++while.body.3: ; preds = %if.end
|
|
|
++ %tobool4.old = icmp ne i32 %d.0, 0, !dbg !29 ; line:16 col:14
|
|
|
++ br i1 %tobool4.old, label %if.end, label %while.end.14, !dbg !30 ; line:16 col:13
|
|
|
++
|
|
|
++if.end: ; preds = %if.end.preheader, %while.body.3
|
|
|
++ %tobool6 = icmp ne i32 %d.0, 0, !dbg !31 ; line:20 col:19
|
|
|
++ %tobool7 = icmp ne i32 %d.0, 0, !dbg !32 ; line:20 col:23
|
|
|
++ %tobool8 = icmp ne i32 %d.0, 0, !dbg !33 ; line:20 col:27
|
|
|
++ %4 = select i1 %tobool6, i1 %tobool7, i1 %tobool8, !dbg !31 ; line:20 col:19
|
|
|
++ br i1 %4, label %while.body.loopexit, label %while.body.3, !dbg !34 ; line:21 col:13
|
|
|
++
|
|
|
++while.end.14: ; preds = %while.body.loopexit, %while.body.3, %entry
|
|
|
++ ret void, !dbg !35 ; line:27 col:1
|
|
|
++}
|
|
|
++
|
|
|
++; Function Attrs: nounwind readnone
|
|
|
++declare %dx.types.Handle @"dx.hl.createhandle..%dx.types.Handle (i32, %cbuffer_g*, i32)"(i32, %cbuffer_g*, i32) #1
|
|
|
++
|
|
|
++; Function Attrs: nounwind readnone
|
|
|
++declare %dx.types.Handle @"dx.hl.annotatehandle..%dx.types.Handle (i32, %dx.types.Handle, %dx.types.ResourceProperties, %cbuffer_g)"(i32, %dx.types.Handle, %dx.types.ResourceProperties, %cbuffer_g) #1
|
|
|
++
|
|
|
++; Function Attrs: nounwind readonly
|
|
|
++declare %dx.types.CBufRet.i32 @dx.op.cbufferLoadLegacy.i32(i32, %dx.types.Handle, i32) #2
|
|
|
++
|
|
|
++; Function Attrs: nounwind readonly
|
|
|
++declare %dx.types.Handle @dx.op.createHandleForLib.cbuffer_g(i32, %cbuffer_g) #2
|
|
|
++
|
|
|
++; Function Attrs: nounwind readnone
|
|
|
++declare %dx.types.Handle @dx.op.annotateHandle(i32, %dx.types.Handle, %dx.types.ResourceProperties) #1
|
|
|
++
|
|
|
++attributes #0 = { nounwind }
|
|
|
++attributes #1 = { nounwind readnone }
|
|
|
++attributes #2 = { nounwind readonly }
|
|
|
++
|
|
|
++!llvm.module.flags = !{!0}
|
|
|
++!pauseresume = !{!1}
|
|
|
++!llvm.ident = !{!2}
|
|
|
++!dx.version = !{!3}
|
|
|
++!dx.valver = !{!4}
|
|
|
++!dx.shaderModel = !{!5}
|
|
|
++!dx.resources = !{!6}
|
|
|
++!dx.typeAnnotations = !{!9, !12}
|
|
|
++!dx.entryPoints = !{!16}
|
|
|
++
|
|
|
++!0 = !{i32 2, !"Debug Info Version", i32 3}
|
|
|
++!1 = !{!"hlsl-dxilemit", !"hlsl-dxilload"}
|
|
|
++!2 = !{!"dxc(private) 1.8.0.4514 (d9bd2a706-dirty)"}
|
|
|
++!3 = !{i32 1, i32 0}
|
|
|
++!4 = !{i32 1, i32 8}
|
|
|
++!5 = !{!"cs", i32 6, i32 0}
|
|
|
++!6 = !{null, null, !7, null}
|
|
|
++!7 = !{!8}
|
|
|
++!8 = !{i32 0, %cbuffer_g* @cbuffer_g, !"cbuffer_g", i32 0, i32 0, i32 1, i32 16, null}
|
|
|
++!9 = !{i32 0, %cbuffer_g undef, !10}
|
|
|
++!10 = !{i32 16, !11}
|
|
|
++!11 = !{i32 6, !"g", i32 3, i32 0, i32 7, i32 5}
|
|
|
++!12 = !{i32 1, void ()* @main, !13}
|
|
|
++!13 = !{!14}
|
|
|
++!14 = !{i32 1, !15, !15}
|
|
|
++!15 = !{}
|
|
|
++!16 = !{void ()* @main, !"main", null, !6, !17}
|
|
|
++!17 = !{i32 4, !18}
|
|
|
++!18 = !{i32 1, i32 1, i32 1}
|
|
|
++!19 = !DILocation(line: 8, column: 13, scope: !20)
|
|
|
++!20 = !DISubprogram(name: "main", scope: !21, file: !21, line: 6, type: !22, isLocal: false, isDefinition: true, scopeLine: 6, flags: DIFlagPrototyped, isOptimized: false, function: void ()* @main)
|
|
|
++!21 = !DIFile(filename: "/mnt/c/Users/amaiorano/Downloads/342545100/standalone_reduced.hlsl", directory: "")
|
|
|
++!22 = !DISubroutineType(types: !15)
|
|
|
++!23 = !DILocation(line: 8, column: 20, scope: !20)
|
|
|
++!24 = !DILocation(line: 9, column: 15, scope: !20)
|
|
|
++!25 = !DILocation(line: 11, column: 17, scope: !20)
|
|
|
++!26 = !DILocation(line: 11, column: 10, scope: !20)
|
|
|
++!27 = !DILocation(line: 12, column: 9, scope: !20)
|
|
|
++!28 = !DILocation(line: 19, column: 13, scope: !20)
|
|
|
++!29 = !DILocation(line: 16, column: 14, scope: !20)
|
|
|
++!30 = !DILocation(line: 16, column: 13, scope: !20)
|
|
|
++!31 = !DILocation(line: 20, column: 19, scope: !20)
|
|
|
++!32 = !DILocation(line: 20, column: 23, scope: !20)
|
|
|
++!33 = !DILocation(line: 20, column: 27, scope: !20)
|
|
|
++!34 = !DILocation(line: 21, column: 13, scope: !20)
|
|
|
++!35 = !DILocation(line: 27, column: 1, scope: !20)
|