[Clang][DirectX] Always use Diagnostic Printer #135655
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Farzon Lotfi (farzonl) Changesfixes #135654 In #128613 we added safe guards to prevent the lowering of just any intrinsic in the backend. We used What we found was when using 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 Full diff: https://github.com/llvm/llvm-project/pull/135655.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp
index 1f5eb427b566f..9d7a8b92f6616 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -605,13 +605,12 @@ void BackendConsumer::UnsupportedDiagHandler(
// Context will be nullptr for IR input files, we will construct the diag
// message from llvm::DiagnosticInfoUnsupported.
- if (Context != nullptr) {
+ if (Context != nullptr)
Loc = getBestLocationFromDebugLoc(D, BadDebugInfo, Filename, Line, Column);
- MsgStream << D.getMessage();
- } else {
- DiagnosticPrinterRawOStream DP(MsgStream);
- D.print(DP);
- }
+
+ DiagnosticPrinterRawOStream DP(MsgStream);
+ D.print(DP);
+
auto DiagType = D.getSeverity() == llvm::DS_Error
? diag::err_fe_backend_unsupported
diff --git a/clang/test/CodeGenDirectX/unsupported_reduc_mul.hlsl b/clang/test/CodeGenDirectX/unsupported_reduc_mul.hlsl
new file mode 100644
index 0000000000000..3d0462679f152
--- /dev/null
+++ b/clang/test/CodeGenDirectX/unsupported_reduc_mul.hlsl
@@ -0,0 +1,10 @@
+// RUN: not %clang_dxc -T lib_6_3 %s 2>&1 | FileCheck %s
+
+// Define a vector of 4 ints (Clang/LLVM extension)
+typedef int int4 __attribute__((ext_vector_type(4)));
+
+// CHECK: error: <unknown>:0:0: in function llvm.vector.reduce.mul.v4i32 i32 (<4 x i32>): Unsupported intrinsic for DXIL lowering
+
+export int vecReduceMulTest(int4 vec) {
+ return __builtin_reduce_mul(vec);
+}
|
|
@llvm/pr-subscribers-clang-codegen Author: Farzon Lotfi (farzonl) Changesfixes #135654 In #128613 we added safe guards to prevent the lowering of just any intrinsic in the backend. We used What we found was when using 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 Full diff: https://github.com/llvm/llvm-project/pull/135655.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp
index 1f5eb427b566f..9d7a8b92f6616 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -605,13 +605,12 @@ void BackendConsumer::UnsupportedDiagHandler(
// Context will be nullptr for IR input files, we will construct the diag
// message from llvm::DiagnosticInfoUnsupported.
- if (Context != nullptr) {
+ if (Context != nullptr)
Loc = getBestLocationFromDebugLoc(D, BadDebugInfo, Filename, Line, Column);
- MsgStream << D.getMessage();
- } else {
- DiagnosticPrinterRawOStream DP(MsgStream);
- D.print(DP);
- }
+
+ DiagnosticPrinterRawOStream DP(MsgStream);
+ D.print(DP);
+
auto DiagType = D.getSeverity() == llvm::DS_Error
? diag::err_fe_backend_unsupported
diff --git a/clang/test/CodeGenDirectX/unsupported_reduc_mul.hlsl b/clang/test/CodeGenDirectX/unsupported_reduc_mul.hlsl
new file mode 100644
index 0000000000000..3d0462679f152
--- /dev/null
+++ b/clang/test/CodeGenDirectX/unsupported_reduc_mul.hlsl
@@ -0,0 +1,10 @@
+// RUN: not %clang_dxc -T lib_6_3 %s 2>&1 | FileCheck %s
+
+// Define a vector of 4 ints (Clang/LLVM extension)
+typedef int int4 __attribute__((ext_vector_type(4)));
+
+// CHECK: error: <unknown>:0:0: in function llvm.vector.reduce.mul.v4i32 i32 (<4 x i32>): Unsupported intrinsic for DXIL lowering
+
+export int vecReduceMulTest(int4 vec) {
+ return __builtin_reduce_mul(vec);
+}
|
|
@llvm/pr-subscribers-backend-directx Author: Farzon Lotfi (farzonl) Changesfixes #135654 In #128613 we added safe guards to prevent the lowering of just any intrinsic in the backend. We used What we found was when using 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 Full diff: https://github.com/llvm/llvm-project/pull/135655.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp
index 1f5eb427b566f..9d7a8b92f6616 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -605,13 +605,12 @@ void BackendConsumer::UnsupportedDiagHandler(
// Context will be nullptr for IR input files, we will construct the diag
// message from llvm::DiagnosticInfoUnsupported.
- if (Context != nullptr) {
+ if (Context != nullptr)
Loc = getBestLocationFromDebugLoc(D, BadDebugInfo, Filename, Line, Column);
- MsgStream << D.getMessage();
- } else {
- DiagnosticPrinterRawOStream DP(MsgStream);
- D.print(DP);
- }
+
+ DiagnosticPrinterRawOStream DP(MsgStream);
+ D.print(DP);
+
auto DiagType = D.getSeverity() == llvm::DS_Error
? diag::err_fe_backend_unsupported
diff --git a/clang/test/CodeGenDirectX/unsupported_reduc_mul.hlsl b/clang/test/CodeGenDirectX/unsupported_reduc_mul.hlsl
new file mode 100644
index 0000000000000..3d0462679f152
--- /dev/null
+++ b/clang/test/CodeGenDirectX/unsupported_reduc_mul.hlsl
@@ -0,0 +1,10 @@
+// RUN: not %clang_dxc -T lib_6_3 %s 2>&1 | FileCheck %s
+
+// Define a vector of 4 ints (Clang/LLVM extension)
+typedef int int4 __attribute__((ext_vector_type(4)));
+
+// CHECK: error: <unknown>:0:0: in function llvm.vector.reduce.mul.v4i32 i32 (<4 x i32>): Unsupported intrinsic for DXIL lowering
+
+export int vecReduceMulTest(int4 vec) {
+ return __builtin_reduce_mul(vec);
+}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
farzonl
force-pushed
the
bugfix/issue-135654
branch
from
4dc1476 to
e370144
Compare
fixes llvm#135654 In llvm#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 printer was called but when using clang the diagnostic message was called. 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`.
farzonl
force-pushed
the
bugfix/issue-135654
branch
from
e370144 to
7ec850f
Compare
| @@ -2,7 +2,6 @@ | |||
| // RUN: not %clang_cc1 %s -o - -S -triple=amdgcn-amd-amdhsa 2>&1 | FileCheck %s | |||
|
|
|||
| void test_amdgcn_fence_failure() { | |||
|
|
|||
| // CHECK: error: Unsupported atomic synchronization scope | |||
| // CHECK: error: <unknown>:0:0: in function _Z25test_amdgcn_fence_failurev void (): Unsupported atomic synchronization scope | |||
There was a problem hiding this comment.
Current diagnostic without debug info:
<stdin>:1:6: error: Unsupported atomic synchronization scope
1 | void test_amdgcn_fence_failure() {
| ^
Current diagnostic with debug info:
<stdin>:3:3: error: Unsupported atomic synchronization scope
3 | __builtin_amdgcn_fence(__ATOMIC_SEQ_CST, "foobar");
| ^
The one without debug info could maybe be improved a bit, but throwing away the location info doesn't seem like the right approach.
There was a problem hiding this comment.
My change doesn't get rid of the location we still prefix it, but <unknown>:0:0 is also prefixed and that I agree is problematic. I suppose this wasn't a problem for the Context==nullptr case because in that on Diags.Report(Loc, DiagType) might be an empty string?
Before change No debug:
<llvm-project_dir>/clang/test/Sema/builtin-amdgcn-fence-failure.cpp:4:6: error: Unsupported atomic synchronization scope
4 | void test_amdgcn_fence_failure() {
| ^
Before change debug
/mnt/DevDrive/projects/llvm-project/clang/test/Sema/builtin-amdgcn-fence-failure.cpp:7:3: error: Unsupported atomic synchronization scope
7 | __builtin_amdgcn_fence(__ATOMIC_SEQ_CST, "foobar");
| ^
After change no debug:
<llvm-project_dir>/clang/test/Sema/builtin-amdgcn-fence-failure.cpp:4:6: error: <unknown>:0:0: in function _Z25test_amdgcn_fence_failurev void (): Unsupported atomic synchronization scope
7 | __builtin_amdgcn_fence(__ATOMIC_SEQ_CST, "foobar");
| ^
After change debug
/mnt/DevDrive/projects/llvm-project/clang/test/Sema/builtin-amdgcn-fence-failure.cpp:7:3: error: clang/test/Sema/builtin-amdgcn-fence-failure.cpp:7:3: in function _Z25test_amdgcn_fence_failurev void (): Unsupported atomic synchronization scope
7 | __builtin_amdgcn_fence(__ATOMIC_SEQ_CST, "foobar");
| ^
| @@ -605,13 +605,11 @@ void BackendConsumer::UnsupportedDiagHandler( | |||
|
|
|||
| // Context will be nullptr for IR input files, we will construct the diag | |||
| // message from llvm::DiagnosticInfoUnsupported. | |||
| if (Context != nullptr) { | |||
| if (Context != nullptr) | |||
| Loc = getBestLocationFromDebugLoc(D, BadDebugInfo, Filename, Line, Column); | |||
There was a problem hiding this comment.
Is the best location always "unknown"? or are these tests only for cases where the context is null?
fixes #135654
In #128613 we added safe guards to prevent the lowering of just any intrinsic in the backend. We used
DiagnosticInfoUnsupportedto do this.What we found was when using
optthe diagnostic printer was called but when using clang the diagnostic message was called.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.