From 36b9faa92a640499290d126ffd8797d545289c0e Mon Sep 17 00:00:00 2001 From: Kevin McAfee Date: Mon, 22 Jul 2024 15:53:56 -0700 Subject: [PATCH 1/7] [SDAG] Read-only intrinsics must have WillReturn to be treated as loads --- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index 7cdd3d47b641d..68f25ff433669 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -5229,7 +5229,7 @@ void SelectionDAGBuilder::visitTargetIntrinsic(const CallInst &I, // definition. const Function *F = I.getCalledFunction(); bool HasChain = !F->doesNotAccessMemory(); - bool OnlyLoad = HasChain && F->onlyReadsMemory(); + bool OnlyLoad = HasChain && F->onlyReadsMemory() && F->willReturn(); // Build the operand list. SmallVector Ops; From 53470ccb6f60dd8cab19d33ab90079b5868e86cb Mon Sep 17 00:00:00 2001 From: Kevin McAfee Date: Mon, 22 Jul 2024 16:00:46 -0700 Subject: [PATCH 2/7] Update BPF test instruction order --- llvm/test/CodeGen/BPF/intrinsics.ll | 8 ++++---- llvm/test/CodeGen/BPF/objdump_intrinsics.ll | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/llvm/test/CodeGen/BPF/intrinsics.ll b/llvm/test/CodeGen/BPF/intrinsics.ll index 0f59fd5604732..8331ee5ff04ee 100644 --- a/llvm/test/CodeGen/BPF/intrinsics.ll +++ b/llvm/test/CodeGen/BPF/intrinsics.ll @@ -31,10 +31,10 @@ define i32 @ld_h(ptr %ctx, ptr %ctx2, i32 %foo) #0 { %5 = trunc i64 %4 to i32 ret i32 %5 ; CHECK-LABEL: ld_h: -; CHECK-EL: r0 = *(u16 *)skb[r ; CHECK-EL: r0 = *(u16 *)skb[123] -; CHECK-EB: r0 = *(u16 *)skb[r +; CHECK-EL: r0 = *(u16 *)skb[r ; CHECK-EB: r0 = *(u16 *)skb[123] +; CHECK-EB: r0 = *(u16 *)skb[r } declare i64 @llvm.bpf.load.half(ptr, i64) #1 @@ -48,10 +48,10 @@ define i32 @ld_w(ptr %ctx, ptr %ctx2, i32 %foo) #0 { %5 = trunc i64 %4 to i32 ret i32 %5 ; CHECK-LABEL: ld_w: -; CHECK-EL: r0 = *(u32 *)skb[r ; CHECK-EL: r0 = *(u32 *)skb[123] -; CHECK-EB: r0 = *(u32 *)skb[r +; CHECK-EL: r0 = *(u32 *)skb[r ; CHECK-EB: r0 = *(u32 *)skb[123] +; CHECK-EB: r0 = *(u32 *)skb[r } declare i64 @llvm.bpf.load.word(ptr, i64) #1 diff --git a/llvm/test/CodeGen/BPF/objdump_intrinsics.ll b/llvm/test/CodeGen/BPF/objdump_intrinsics.ll index 92db0882398b6..5497fce989d21 100644 --- a/llvm/test/CodeGen/BPF/objdump_intrinsics.ll +++ b/llvm/test/CodeGen/BPF/objdump_intrinsics.ll @@ -31,10 +31,10 @@ define i32 @ld_h(ptr %ctx, ptr %ctx2, i32 %foo) #0 { %5 = trunc i64 %4 to i32 ret i32 %5 ; CHECK-LABEL: ld_h: -; CHECK-EL: r0 = *(u16 *)skb[r ; CHECK-EL: r0 = *(u16 *)skb[123] -; CHECK-EB: r0 = *(u16 *)skb[r +; CHECK-EL: r0 = *(u16 *)skb[r ; CHECK-EB: r0 = *(u16 *)skb[123] +; CHECK-EB: r0 = *(u16 *)skb[r } declare i64 @llvm.bpf.load.half(ptr, i64) #1 @@ -48,10 +48,10 @@ define i32 @ld_w(ptr %ctx, ptr %ctx2, i32 %foo) #0 { %5 = trunc i64 %4 to i32 ret i32 %5 ; CHECK-LABEL: ld_w: -; CHECK-EL: r0 = *(u32 *)skb[r ; CHECK-EL: r0 = *(u32 *)skb[123] -; CHECK-EB: r0 = *(u32 *)skb[r +; CHECK-EL: r0 = *(u32 *)skb[r ; CHECK-EB: r0 = *(u32 *)skb[123] +; CHECK-EB: r0 = *(u32 *)skb[r } declare i64 @llvm.bpf.load.word(ptr, i64) #1 From 69ce7520da1d00b48aed98de4bd9732ee9c6e98d Mon Sep 17 00:00:00 2001 From: Kevin McAfee Date: Wed, 31 Jul 2024 14:52:03 -0700 Subject: [PATCH 3/7] Temp assert to find tests with non-WillReturn intrinsics --- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index 68f25ff433669..30eff3b3ae2b6 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -5231,6 +5231,9 @@ void SelectionDAGBuilder::visitTargetIntrinsic(const CallInst &I, bool HasChain = !F->doesNotAccessMemory(); bool OnlyLoad = HasChain && F->onlyReadsMemory() && F->willReturn(); + bool PrevOnlyLoad = HasChain && F->onlyReadsMemory(); + assert((!PrevOnlyLoad || F->willReturn()) && "Intrinsic treated as `OnlyLoad` despite no WillReturn"); + // Build the operand list. SmallVector Ops; if (HasChain) { // If this intrinsic has side-effects, chainify it. From 3d654ed054b55cc45c7999393e36765e8d8d7bc3 Mon Sep 17 00:00:00 2001 From: Kevin McAfee Date: Wed, 31 Jul 2024 16:56:02 -0700 Subject: [PATCH 4/7] Revert "Update BPF test instruction order" This reverts commit fa37027fe1b0c930ae4b83b6ca4835797fa719e3. --- llvm/test/CodeGen/BPF/intrinsics.ll | 8 ++++---- llvm/test/CodeGen/BPF/objdump_intrinsics.ll | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/llvm/test/CodeGen/BPF/intrinsics.ll b/llvm/test/CodeGen/BPF/intrinsics.ll index 8331ee5ff04ee..0f59fd5604732 100644 --- a/llvm/test/CodeGen/BPF/intrinsics.ll +++ b/llvm/test/CodeGen/BPF/intrinsics.ll @@ -31,10 +31,10 @@ define i32 @ld_h(ptr %ctx, ptr %ctx2, i32 %foo) #0 { %5 = trunc i64 %4 to i32 ret i32 %5 ; CHECK-LABEL: ld_h: -; CHECK-EL: r0 = *(u16 *)skb[123] ; CHECK-EL: r0 = *(u16 *)skb[r -; CHECK-EB: r0 = *(u16 *)skb[123] +; CHECK-EL: r0 = *(u16 *)skb[123] ; CHECK-EB: r0 = *(u16 *)skb[r +; CHECK-EB: r0 = *(u16 *)skb[123] } declare i64 @llvm.bpf.load.half(ptr, i64) #1 @@ -48,10 +48,10 @@ define i32 @ld_w(ptr %ctx, ptr %ctx2, i32 %foo) #0 { %5 = trunc i64 %4 to i32 ret i32 %5 ; CHECK-LABEL: ld_w: -; CHECK-EL: r0 = *(u32 *)skb[123] ; CHECK-EL: r0 = *(u32 *)skb[r -; CHECK-EB: r0 = *(u32 *)skb[123] +; CHECK-EL: r0 = *(u32 *)skb[123] ; CHECK-EB: r0 = *(u32 *)skb[r +; CHECK-EB: r0 = *(u32 *)skb[123] } declare i64 @llvm.bpf.load.word(ptr, i64) #1 diff --git a/llvm/test/CodeGen/BPF/objdump_intrinsics.ll b/llvm/test/CodeGen/BPF/objdump_intrinsics.ll index 5497fce989d21..92db0882398b6 100644 --- a/llvm/test/CodeGen/BPF/objdump_intrinsics.ll +++ b/llvm/test/CodeGen/BPF/objdump_intrinsics.ll @@ -31,10 +31,10 @@ define i32 @ld_h(ptr %ctx, ptr %ctx2, i32 %foo) #0 { %5 = trunc i64 %4 to i32 ret i32 %5 ; CHECK-LABEL: ld_h: -; CHECK-EL: r0 = *(u16 *)skb[123] ; CHECK-EL: r0 = *(u16 *)skb[r -; CHECK-EB: r0 = *(u16 *)skb[123] +; CHECK-EL: r0 = *(u16 *)skb[123] ; CHECK-EB: r0 = *(u16 *)skb[r +; CHECK-EB: r0 = *(u16 *)skb[123] } declare i64 @llvm.bpf.load.half(ptr, i64) #1 @@ -48,10 +48,10 @@ define i32 @ld_w(ptr %ctx, ptr %ctx2, i32 %foo) #0 { %5 = trunc i64 %4 to i32 ret i32 %5 ; CHECK-LABEL: ld_w: -; CHECK-EL: r0 = *(u32 *)skb[123] ; CHECK-EL: r0 = *(u32 *)skb[r -; CHECK-EB: r0 = *(u32 *)skb[123] +; CHECK-EL: r0 = *(u32 *)skb[123] ; CHECK-EB: r0 = *(u32 *)skb[r +; CHECK-EB: r0 = *(u32 *)skb[123] } declare i64 @llvm.bpf.load.word(ptr, i64) #1 From ac4be45035521da0fce95e7387118f80de56c938 Mon Sep 17 00:00:00 2001 From: Kevin McAfee Date: Thu, 15 Aug 2024 17:04:49 -0700 Subject: [PATCH 5/7] Revert "Temp assert to find tests with non-WillReturn intrinsics" This reverts commit 69ce7520da1d00b48aed98de4bd9732ee9c6e98d. --- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index 30eff3b3ae2b6..68f25ff433669 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -5231,9 +5231,6 @@ void SelectionDAGBuilder::visitTargetIntrinsic(const CallInst &I, bool HasChain = !F->doesNotAccessMemory(); bool OnlyLoad = HasChain && F->onlyReadsMemory() && F->willReturn(); - bool PrevOnlyLoad = HasChain && F->onlyReadsMemory(); - assert((!PrevOnlyLoad || F->willReturn()) && "Intrinsic treated as `OnlyLoad` despite no WillReturn"); - // Build the operand list. SmallVector Ops; if (HasChain) { // If this intrinsic has side-effects, chainify it. From c25bdf123276450be38db410502547902c4e944a Mon Sep 17 00:00:00 2001 From: Kevin McAfee Date: Fri, 16 Aug 2024 09:31:09 -0700 Subject: [PATCH 6/7] Require F->doesNotThrow for OnlyLoad --- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index 68f25ff433669..f93eba2298227 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -5229,7 +5229,7 @@ void SelectionDAGBuilder::visitTargetIntrinsic(const CallInst &I, // definition. const Function *F = I.getCalledFunction(); bool HasChain = !F->doesNotAccessMemory(); - bool OnlyLoad = HasChain && F->onlyReadsMemory() && F->willReturn(); + bool OnlyLoad = HasChain && F->onlyReadsMemory() && F->willReturn() && F->doesNotThrow(); // Build the operand list. SmallVector Ops; From 0ee549c3030e6d834852d35efce4fc9dc3617013 Mon Sep 17 00:00:00 2001 From: Kevin McAfee Date: Fri, 16 Aug 2024 09:34:46 -0700 Subject: [PATCH 7/7] Format --- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index f93eba2298227..2f1b0cb7ade3c 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -5229,7 +5229,8 @@ void SelectionDAGBuilder::visitTargetIntrinsic(const CallInst &I, // definition. const Function *F = I.getCalledFunction(); bool HasChain = !F->doesNotAccessMemory(); - bool OnlyLoad = HasChain && F->onlyReadsMemory() && F->willReturn() && F->doesNotThrow(); + bool OnlyLoad = + HasChain && F->onlyReadsMemory() && F->willReturn() && F->doesNotThrow(); // Build the operand list. SmallVector Ops;