Skip to content

Commit 111af76

Browse files
authored
[DirectX] add Function name to DiagnosticInfoUnsupported Msg in DXILOpLowering (#136234)
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 by line\col number. We would also need to change from iterating on every function to doing something like a LazyCallGraph which is a nonstarter. 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 a combo of option 2 & 5. Its low code change that also only impacts the DirectX backend.
1 parent f12078e commit 111af76

File tree

3 files changed

+13
-5
lines changed

3 files changed

+13
-5
lines changed
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// REQUIRES: directx-registered-target
2+
// RUN: not %clang_dxc -T lib_6_3 %s 2>&1 | FileCheck %s
3+
4+
// CHECK: error: Unsupported intrinsic llvm.vector.reduce.and.v4i32 for DXIL lowering
5+
6+
export int vecReduceAndTest(int4 vec) {
7+
return __builtin_reduce_and(vec);
8+
}

llvm/lib/Target/DirectX/DXILOpLowering.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
#include "DXILOpLowering.h"
1010
#include "DXILConstants.h"
11-
#include "DXILIntrinsicExpansion.h"
1211
#include "DXILOpBuilder.h"
1312
#include "DXILShaderFlags.h"
1413
#include "DirectX.h"
@@ -27,6 +26,7 @@
2726
#include "llvm/InitializePasses.h"
2827
#include "llvm/Pass.h"
2928
#include "llvm/Support/ErrorHandling.h"
29+
#include "llvm/Support/FormatVariadic.h"
3030

3131
#define DEBUG_TYPE "dxil-op-lower"
3232

@@ -756,9 +756,9 @@ class OpLowerer {
756756
case Intrinsic::not_intrinsic:
757757
continue;
758758
default: {
759-
DiagnosticInfoUnsupported Diag(
760-
F, "Unsupported intrinsic for DXIL lowering");
761-
M.getContext().diagnose(Diag);
759+
SmallString<128> Msg =
760+
formatv("Unsupported intrinsic {0} for DXIL lowering", F.getName());
761+
M.getContext().emitError(Msg);
762762
HasErrors |= true;
763763
break;
764764
}

llvm/test/CodeGen/DirectX/unsupported_intrinsic.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
; RUN: not opt -S -dxil-op-lower -mtriple=dxil-pc-shadermodel6.3-library %s 2>&1 | FileCheck %s
22

3-
; CHECK: error: <unknown>:0:0: in function llvm.vector.reduce.and.v4i32 i32 (<4 x i32>): Unsupported intrinsic for DXIL lowering
3+
; CHECK: error: Unsupported intrinsic llvm.vector.reduce.and.v4i32 for DXIL lowering
44
define i32 @fn_and(<4 x i32> %0) local_unnamed_addr #0 {
55
%2 = tail call i32 @llvm.vector.reduce.and.v4i32(<4 x i32> %0)
66
ret i32 %2

0 commit comments

Comments
 (0)