https://github.com/farzonl updated
https://github.com/llvm/llvm-project/pull/136234
>From 562084ce64581b1cffe30024082f9eb84393875c Mon Sep 17 00:00:00 2001
From: Farzon Lotfi <farzonlo...@microsoft.com>
Date: Thu, 17 Apr 2025 17:42:34 -0700
Subject: [PATCH 1/3] [DirectX] add Function name to DiagnosticInfoUnsupported
Msg in DXILOpLowering
fixes #135654
In #128613 we added safe guards to prevent the lowering of just any intrinsic
in the backend. We used `DiagnosticInfoUnsupported` to do this.
What we found was when using `opt` the diagnostic print function was called
but when using clang the diagnostic message was used.
Printing message in the clang version means we miss valuable debugging
information like function name and function type when LLVMContext was only
needed to call `getBestLocationFromDebugLoc`.
There are a few potential fixes
1. Write a custom DiagnosticInfoUnsupported so we can change the Message
just for DirectX. Too heavy handed so rejected.
2. Add the function name to the Message in DirectX code. Very simple one
line change. Downside is
when using opt you see the function name twice. But makes the
clang-dxc bugs more actionable.
3. change CodeGenAction.cpp to always use the print function and not the
message directly. Downside is a bunch of innacurate information shows
up in the message if you don't specify
`-debug-info-kind=standalone`.
4. add some book keeping to know which function called the intrinsic
keep a map of these so we can pass the calling function to
`DiagnosticInfoUnsupported` instead of the intrinsic.
This would only be useful if we had debug info so we could
distinguish different uses of the intrinsic. We would also need to
change from iterating on every function to doing somethin like a
LazyCallGraph.
5. pick a different means of doing a Diagnostic error, because other
uses of`DiagnosticInfoUnsupported` error when we are in the body of a
function not when we see one being used like in the intrinsic case.
This PR went with option 2. Its low code change that also only impacts
the DirectX backend.
---
clang/test/CodeGenDirectX/unsupported_intrinsic.hlsl | 7 +++++++
llvm/lib/Target/DirectX/DXILOpLowering.cpp | 9 ++++++---
llvm/test/CodeGen/DirectX/unsupported_intrinsic.ll | 2 +-
3 files changed, 14 insertions(+), 4 deletions(-)
create mode 100644 clang/test/CodeGenDirectX/unsupported_intrinsic.hlsl
diff --git a/clang/test/CodeGenDirectX/unsupported_intrinsic.hlsl
b/clang/test/CodeGenDirectX/unsupported_intrinsic.hlsl
new file mode 100644
index 0000000000000..b10fb5d409f91
--- /dev/null
+++ b/clang/test/CodeGenDirectX/unsupported_intrinsic.hlsl
@@ -0,0 +1,7 @@
+// RUN: %if clang-dxc %{not %clang_dxc -T lib_6_3 %s 2>&1 | FileCheck %s %}
+
+// CHECK: error: Unsupported intrinsic llvm.vector.reduce.and.v4i32 for DXIL
lowering
+
+export int vecReduceAndTest(int4 vec) {
+ return __builtin_reduce_and(vec);
+}
diff --git a/llvm/lib/Target/DirectX/DXILOpLowering.cpp
b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
index 4574e5f7bbd96..3ba963a5b4f8a 100644
--- a/llvm/lib/Target/DirectX/DXILOpLowering.cpp
+++ b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
@@ -8,7 +8,6 @@
#include "DXILOpLowering.h"
#include "DXILConstants.h"
-#include "DXILIntrinsicExpansion.h"
#include "DXILOpBuilder.h"
#include "DXILShaderFlags.h"
#include "DirectX.h"
@@ -27,6 +26,7 @@
#include "llvm/InitializePasses.h"
#include "llvm/Pass.h"
#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FormatVariadic.h"
#define DEBUG_TYPE "dxil-op-lower"
@@ -756,8 +756,11 @@ class OpLowerer {
case Intrinsic::not_intrinsic:
continue;
default: {
- DiagnosticInfoUnsupported Diag(
- F, "Unsupported intrinsic for DXIL lowering");
+ std::string Msg =
+ llvm::formatv("Unsupported intrinsic {0} for DXIL lowering",
+ F.getName())
+ .str();
+ DiagnosticInfoUnsupported Diag(F, Msg);
M.getContext().diagnose(Diag);
HasErrors |= true;
break;
diff --git a/llvm/test/CodeGen/DirectX/unsupported_intrinsic.ll
b/llvm/test/CodeGen/DirectX/unsupported_intrinsic.ll
index d703f2f04c494..3ee5ff89b02a2 100644
--- a/llvm/test/CodeGen/DirectX/unsupported_intrinsic.ll
+++ b/llvm/test/CodeGen/DirectX/unsupported_intrinsic.ll
@@ -1,6 +1,6 @@
; RUN: not opt -S -dxil-op-lower -mtriple=dxil-pc-shadermodel6.3-library %s
2>&1 | FileCheck %s
-; CHECK: error: <unknown>:0:0: in function llvm.vector.reduce.and.v4i32 i32
(<4 x i32>): Unsupported intrinsic for DXIL lowering
+; CHECK: Unsupported intrinsic llvm.vector.reduce.and.v4i32 for DXIL lowering
define i32 @fn_and(<4 x i32> %0) local_unnamed_addr #0 {
%2 = tail call i32 @llvm.vector.reduce.and.v4i32(<4 x i32> %0)
ret i32 %2
>From e257a379412fb6ad80afb209d4cdafe3a1eff2c1 Mon Sep 17 00:00:00 2001
From: Farzon Lotfi <farzonlo...@microsoft.com>
Date: Fri, 18 Apr 2025 10:26:21 -0700
Subject: [PATCH 2/3] address pr comments
---
llvm/lib/Target/DirectX/DXILOpLowering.cpp | 5 ++---
llvm/test/CodeGen/DirectX/unsupported_intrinsic.ll | 2 +-
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Target/DirectX/DXILOpLowering.cpp
b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
index 3ba963a5b4f8a..3110b322a9142 100644
--- a/llvm/lib/Target/DirectX/DXILOpLowering.cpp
+++ b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
@@ -757,10 +757,9 @@ class OpLowerer {
continue;
default: {
std::string Msg =
- llvm::formatv("Unsupported intrinsic {0} for DXIL lowering",
- F.getName())
+ formatv("Unsupported intrinsic {0} for DXIL lowering", F.getName())
.str();
- DiagnosticInfoUnsupported Diag(F, Msg);
+ DiagnosticInfoGeneric Diag(Msg);
M.getContext().diagnose(Diag);
HasErrors |= true;
break;
diff --git a/llvm/test/CodeGen/DirectX/unsupported_intrinsic.ll
b/llvm/test/CodeGen/DirectX/unsupported_intrinsic.ll
index 3ee5ff89b02a2..aaf17d901aef7 100644
--- a/llvm/test/CodeGen/DirectX/unsupported_intrinsic.ll
+++ b/llvm/test/CodeGen/DirectX/unsupported_intrinsic.ll
@@ -1,6 +1,6 @@
; RUN: not opt -S -dxil-op-lower -mtriple=dxil-pc-shadermodel6.3-library %s
2>&1 | FileCheck %s
-; CHECK: Unsupported intrinsic llvm.vector.reduce.and.v4i32 for DXIL lowering
+; CHECK: error: Unsupported intrinsic llvm.vector.reduce.and.v4i32 for DXIL
lowering
define i32 @fn_and(<4 x i32> %0) local_unnamed_addr #0 {
%2 = tail call i32 @llvm.vector.reduce.and.v4i32(<4 x i32> %0)
ret i32 %2
>From 3217f063ab97e78caeb40ef63d3c086a87efa6de Mon Sep 17 00:00:00 2001
From: Farzon Lotfi <farzonlo...@microsoft.com>
Date: Fri, 18 Apr 2025 11:43:53 -0700
Subject: [PATCH 3/3] fix string lifetime issue
---
llvm/include/llvm/IR/DiagnosticInfo.h | 2 +-
llvm/lib/Target/DirectX/DXILOpLowering.cpp | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/llvm/include/llvm/IR/DiagnosticInfo.h
b/llvm/include/llvm/IR/DiagnosticInfo.h
index 8743f8058c382..b99bb133c0a20 100644
--- a/llvm/include/llvm/IR/DiagnosticInfo.h
+++ b/llvm/include/llvm/IR/DiagnosticInfo.h
@@ -139,7 +139,7 @@ class DiagnosticInfo {
using DiagnosticHandlerFunction = std::function<void(const DiagnosticInfo &)>;
class DiagnosticInfoGeneric : public DiagnosticInfo {
- const Twine &MsgStr;
+ const Twine MsgStr;
const Instruction *Inst = nullptr;
public:
diff --git a/llvm/lib/Target/DirectX/DXILOpLowering.cpp
b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
index 3110b322a9142..659359efa6700 100644
--- a/llvm/lib/Target/DirectX/DXILOpLowering.cpp
+++ b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
@@ -759,8 +759,7 @@ class OpLowerer {
std::string Msg =
formatv("Unsupported intrinsic {0} for DXIL lowering", F.getName())
.str();
- DiagnosticInfoGeneric Diag(Msg);
- M.getContext().diagnose(Diag);
+ M.getContext().emitError(Msg);
HasErrors |= true;
break;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
