mirror of https://github.com/electron/electron
232 lines
10 KiB
Diff
232 lines
10 KiB
Diff
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
From: Antonio Maiorano <amaiorano@google.com>
|
|
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 <dsinclair@google.com>
|
|
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
|
|
|
|
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
|
|
+@llvm.used = 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)
|