Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: cherry-pick 2 changes from 2-M124 #42008

Merged
merged 2 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions patches/DirectXShaderCompiler/.patches
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
fix_hlmatrixlowerpass_leaving_call_to_dangling_functionval.patch
cherry-pick-a65e511a14b4.patch
cherry-pick-bc18aec94c82.patch
cherry-pick-bd7aa9779873.patch
161 changes: 161 additions & 0 deletions patches/DirectXShaderCompiler/cherry-pick-bd7aa9779873.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Antonio Maiorano <[email protected]>
Date: Thu, 25 Apr 2024 16:49:11 -0400
Subject: Fixed crash in loop unroll caused by bug in structurize loop exits
(#6548)

Fixed a bug in `hlsl::RemoveUnstructuredLoopExits` where when a new
exiting block is created from splitting, it was added to the current
loop being processed, when it could also part of an inner loop. Not
adding the new block to inner loops that it's part of makes the inner
loops malformed, and causes crash.

This fix adds the new block to the inner most loop that it should be
part of. Also adds the `StructurizeLoopExits` option to `loop-unroll`
pass, which was missing before.

Bug: chromium:333508731
Change-Id: I7efc21bc61aeb81b4906a600c35272af232710ea
Reviewed-on: https://chromium-review.googlesource.com/c/external/github.com/microsoft/DirectXShaderCompiler/+/5490380
Reviewed-by: James Price <[email protected]>
Reviewed-by: Ben Clayton <[email protected]>

diff --git a/lib/Transforms/Scalar/DxilRemoveUnstructuredLoopExits.cpp b/lib/Transforms/Scalar/DxilRemoveUnstructuredLoopExits.cpp
index b6a07d6b27a23ee3831e84cee82299d6d405a288..ef6718f0f22ee33e3f16f9801a64c1a6fb6c653a 100644
--- a/lib/Transforms/Scalar/DxilRemoveUnstructuredLoopExits.cpp
+++ b/lib/Transforms/Scalar/DxilRemoveUnstructuredLoopExits.cpp
@@ -447,7 +447,12 @@ static bool RemoveUnstructuredLoopExitsIteration(BasicBlock *exiting_block,
new_exiting_block->splitBasicBlock(new_exiting_block->getFirstNonPHI());
new_exiting_block->setName("dx.struct_exit.new_exiting");
new_not_exiting_block->setName(old_name);
- L->addBasicBlockToLoop(new_not_exiting_block, *LI);
+ // Query for new_exiting_block's own loop to add new_not_exiting_block to.
+ // It's possible that new_exiting_block is part of another inner loop
+ // separate from L. If added directly to L, the inner loop(s) will not
+ // contain new_not_exiting_block, making them malformed.
+ Loop *inner_loop_of_exiting_block = LI->getLoopFor(new_exiting_block);
+ inner_loop_of_exiting_block->addBasicBlockToLoop(new_not_exiting_block, *LI);

// Branch to latch_exit
new_exiting_block->getTerminator()->eraseFromParent();
diff --git a/lib/Transforms/Scalar/LoopUnrollPass.cpp b/lib/Transforms/Scalar/LoopUnrollPass.cpp
index dd520f7e57d25311be7f3773849a00efaabe6717..b17a5a4a0bc368f16020c4153370ea2c92e5c26c 100644
--- a/lib/Transforms/Scalar/LoopUnrollPass.cpp
+++ b/lib/Transforms/Scalar/LoopUnrollPass.cpp
@@ -155,6 +155,18 @@ namespace {
bool UserAllowPartial;
bool UserRuntime;

+ // HLSL Change - begin
+ // Function overrides that resolve options when used for DxOpt
+ void applyOptions(PassOptions O) override {
+ GetPassOptionBool(O, "StructurizeLoopExits", &StructurizeLoopExits,
+ false);
+ }
+ void dumpConfig(raw_ostream &OS) override {
+ LoopPass::dumpConfig(OS);
+ OS << ",StructurizeLoopExits=" << StructurizeLoopExits;
+ }
+ // HLSL Change - end
+
bool runOnLoop(Loop *L, LPPassManager &LPM) override;

/// This transformation requires natural loop information & requires that
diff --git a/tools/clang/test/DXC/loop_structurize_exit_inner_latch_regression.ll b/tools/clang/test/DXC/loop_structurize_exit_inner_latch_regression.ll
new file mode 100644
index 0000000000000000000000000000000000000000..743135541cd8faec287164ba3b321a59432832b6
--- /dev/null
+++ b/tools/clang/test/DXC/loop_structurize_exit_inner_latch_regression.ll
@@ -0,0 +1,75 @@
+; RUN: %dxopt %s -S -loop-unroll,StructurizeLoopExits=1 | FileCheck %s
+; RUN: %dxopt %s -S -dxil-loop-unroll,StructurizeLoopExits=1 | FileCheck %s
+
+; CHECK: mul nsw i32
+; CHECK: mul nsw i32
+; CHECK-NOT: mul nsw i32
+
+; This is a regression test for a crash in loop unroll. When there are multiple
+; exits, the compiler will run hlsl::RemoveUnstructuredLoopExits to try to
+; avoid unstructured code.
+;
+; In this test, the compiler will try to unroll the middle loop. The exit edge
+; from %land.lhs.true to %if.then will be removed, and %if.end will be split at
+; the beginning, and branch to %if.end instead.
+;
+; Since the new split block at %if.end becomes the new latch of the inner-most
+; loop, it needs to be added to the Loop analysis structure of the inner loop.
+; However, it was only added to the current middle loop that is being unrolled.
+
+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"
+
+; Function Attrs: nounwind
+define void @main(i32 *%arg0, i32 *%arg1, i32 *%arg2) #0 {
+entry:
+ br label %while.body.3.preheader.lr.ph
+
+while.body.3.preheader.lr.ph.loopexit: ; preds = %for.inc
+ br label %while.body.3.preheader.lr.ph
+
+while.body.3.preheader.lr.ph: ; preds = %while.body.3.preheader.lr.ph.loopexit, %entry
+ br label %while.body.3.preheader
+
+while.body.3.preheader: ; preds = %while.body.3.preheader.lr.ph, %for.inc
+ %i.0 = phi i32 [ 0, %while.body.3.preheader.lr.ph ], [ %inc, %for.inc ]
+ br label %while.body.3
+
+while.body.3: ; preds = %while.body.3.preheader, %if.end
+ %load_arg0 = load i32, i32* %arg0
+ %cmp4 = icmp sgt i32 %load_arg0, 0
+ br i1 %cmp4, label %land.lhs.true, label %if.end
+
+land.lhs.true: ; preds = %while.body.3
+ %load_arg1 = load i32, i32* %arg1
+ %load_arg2 = load i32, i32* %arg2
+ %mul = mul nsw i32 %load_arg2, %load_arg1
+ %cmp7 = icmp eq i32 %mul, 10
+ br i1 %cmp7, label %if.then, label %if.end
+
+if.then: ; preds = %land.lhs.true
+ ret void
+
+if.end: ; preds = %land.lhs.true, %while.body.3
+ %cmp10 = icmp sle i32 %i.0, 4
+ br i1 %cmp10, label %for.inc, label %while.body.3
+
+for.inc: ; preds = %if.end
+ %inc = add nsw i32 %i.0, 1
+ %cmp = icmp slt i32 %inc, 2
+ br i1 %cmp, label %while.body.3.preheader, label %while.body.3.preheader.lr.ph.loopexit, !llvm.loop !3
+}
+
+attributes #0 = { nounwind }
+attributes #1 = { nounwind readnone }
+attributes #2 = { nounwind readonly }
+
+!llvm.module.flags = !{!0}
+!pauseresume = !{!1}
+!llvm.ident = !{!2}
+
+!0 = !{i32 2, !"Debug Info Version", i32 3}
+!1 = !{!"hlsl-dxilemit", !"hlsl-dxilload"}
+!2 = !{!"dxc(private) 1.8.0.14563 (main, 07ce88034-dirty)"}
+!3 = distinct !{!3, !4}
+!4 = !{!"llvm.loop.unroll.full"}
diff --git a/utils/hct/hctdb.py b/utils/hct/hctdb.py
index 578b8ec99586b8a2a060355f3758d89522f67623..bcd2a3b9053e62bfeddbfb11f89403adfbde604e 100644
--- a/utils/hct/hctdb.py
+++ b/utils/hct/hctdb.py
@@ -5908,6 +5908,12 @@ class db_dxil(object):
"t": "unsigned",
"d": "Unrolled size limit for loops with an unroll(full) or unroll_count pragma.",
},
+ {
+ "n": "StructurizeLoopExits",
+ "t": "bool",
+ "c": 1,
+ "d": "Whether the unroller should try to structurize loop exits first.",
+ },
],
)
add_pass("mldst-motion", "MergedLoadStoreMotion", "MergedLoadStoreMotion", [])
1 change: 1 addition & 0 deletions patches/chromium/.patches
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,4 @@ m122_webcodecs_disable_async_videoframe_readback_to_mitigate_a.patch
fix_paintimage_deserialization_arbitrary-read_issue.patch
reland_sensors_winrt_call_onreadingchangedcallback_via.patch
cherry-pick-1b1f34234346.patch
cherry-pick-98bcf9ef5cdd.patch
102 changes: 102 additions & 0 deletions patches/chromium/cherry-pick-98bcf9ef5cdd.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Tommy Steimel <[email protected]>
Date: Tue, 23 Apr 2024 19:29:23 +0000
Subject: Don't assume the enter event window is a LocalDOMWindow

This CL changes DocumentPictureInPictureEvent to store a DOMWindow
instead of a LocalDOMWindow to prevent crashes when the window it gets
is actually a RemoteDOMWindow.

(cherry picked from commit 2314741cdf2c4a6e11234dda7006ec0dd9005bbb)

Bug: 335003891
Change-Id: I86a0ec5a89b51a26d5dd89559f86e6e4d6c3e8fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5467978
Commit-Queue: Tommy Steimel <[email protected]>
Reviewed-by: Frank Liberato <[email protected]>
Cr-Original-Commit-Position: refs/heads/main@{#1290122}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5477908
Auto-Submit: Tommy Steimel <[email protected]>
Commit-Queue: Frank Liberato <[email protected]>
Cr-Commit-Position: refs/branch-heads/6367@{#974}
Cr-Branched-From: d158c6dc6e3604e6f899041972edf26087a49740-refs/heads/main@{#1274542}

diff --git a/third_party/blink/renderer/modules/document_picture_in_picture/document_picture_in_picture_event.cc b/third_party/blink/renderer/modules/document_picture_in_picture/document_picture_in_picture_event.cc
index 037813c62c2f0dfc78b3451320a799a349ffde23..572d0803c25a99ef5dfd631e7872b05a681f0444 100644
--- a/third_party/blink/renderer/modules/document_picture_in_picture/document_picture_in_picture_event.cc
+++ b/third_party/blink/renderer/modules/document_picture_in_picture/document_picture_in_picture_event.cc
@@ -8,7 +8,7 @@ namespace blink {

DocumentPictureInPictureEvent* DocumentPictureInPictureEvent::Create(
const AtomicString& type,
- LocalDOMWindow* document_picture_in_picture_window) {
+ DOMWindow* document_picture_in_picture_window) {
return MakeGarbageCollected<DocumentPictureInPictureEvent>(
type, document_picture_in_picture_window);
}
@@ -19,13 +19,13 @@ DocumentPictureInPictureEvent* DocumentPictureInPictureEvent::Create(
return MakeGarbageCollected<DocumentPictureInPictureEvent>(type, initializer);
}

-LocalDOMWindow* DocumentPictureInPictureEvent::window() const {
+DOMWindow* DocumentPictureInPictureEvent::window() const {
return document_picture_in_picture_window_.Get();
}

DocumentPictureInPictureEvent::DocumentPictureInPictureEvent(
AtomicString const& type,
- LocalDOMWindow* document_picture_in_picture_window)
+ DOMWindow* document_picture_in_picture_window)
: Event(type, Bubbles::kYes, Cancelable::kNo),
document_picture_in_picture_window_(document_picture_in_picture_window) {}

@@ -33,8 +33,7 @@ DocumentPictureInPictureEvent::DocumentPictureInPictureEvent(
AtomicString const& type,
const DocumentPictureInPictureEventInit* initializer)
: Event(type, initializer),
- document_picture_in_picture_window_(
- static_cast<LocalDOMWindow*>(initializer->window())) {}
+ document_picture_in_picture_window_(initializer->window()) {}

void DocumentPictureInPictureEvent::Trace(Visitor* visitor) const {
visitor->Trace(document_picture_in_picture_window_);
diff --git a/third_party/blink/renderer/modules/document_picture_in_picture/document_picture_in_picture_event.h b/third_party/blink/renderer/modules/document_picture_in_picture/document_picture_in_picture_event.h
index 7af2022146905a3c3d71e1420aaa68da30e6a9ce..59cd8cb7a2e3a2e2a81db1d146f8075b13755c0e 100644
--- a/third_party/blink/renderer/modules/document_picture_in_picture/document_picture_in_picture_event.h
+++ b/third_party/blink/renderer/modules/document_picture_in_picture/document_picture_in_picture_event.h
@@ -6,7 +6,7 @@
#define THIRD_PARTY_BLINK_RENDERER_MODULES_DOCUMENT_PICTURE_IN_PICTURE_DOCUMENT_PICTURE_IN_PICTURE_EVENT_H_

#include "third_party/blink/renderer/bindings/modules/v8/v8_document_picture_in_picture_event_init.h"
-#include "third_party/blink/renderer/core/frame/local_dom_window.h"
+#include "third_party/blink/renderer/core/frame/dom_window.h"
#include "third_party/blink/renderer/modules/event_modules.h"
#include "third_party/blink/renderer/platform/wtf/text/atomic_string.h"

@@ -18,22 +18,21 @@ class MODULES_EXPORT DocumentPictureInPictureEvent final : public Event {
DEFINE_WRAPPERTYPEINFO();

public:
- static DocumentPictureInPictureEvent* Create(const AtomicString&,
- LocalDOMWindow*);
+ static DocumentPictureInPictureEvent* Create(const AtomicString&, DOMWindow*);
static DocumentPictureInPictureEvent* Create(
const AtomicString&,
const DocumentPictureInPictureEventInit*);

- DocumentPictureInPictureEvent(AtomicString const&, LocalDOMWindow*);
+ DocumentPictureInPictureEvent(AtomicString const&, DOMWindow*);
DocumentPictureInPictureEvent(AtomicString const&,
const DocumentPictureInPictureEventInit*);

- LocalDOMWindow* window() const;
+ DOMWindow* window() const;

void Trace(Visitor*) const override;

private:
- Member<LocalDOMWindow> document_picture_in_picture_window_;
+ Member<DOMWindow> document_picture_in_picture_window_;
};

} // namespace blink