[PATCH] D121549: Define ABI breaking class members correctly

[Daniil Kovalev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf854434f0f2a: [NVPTX] Enhance vectorization of ld.param 
& st.param (authored by kovdan01).
Herald added subscribers: asavonic, StephenFan, jholewinski.
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D121549?vs=415049&id=417855#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121549/new/

https://reviews.llvm.org/D121549

Files:
  clang/test/CodeGenCUDA/device-fun-linkage.cu
  llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
  llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
  llvm/lib/Target/NVPTX/NVPTXISelLowering.h
  llvm/lib/Target/NVPTX/NVPTXLowerArgs.cpp
  llvm/test/CodeGen/NVPTX/param-vectorize-device.ll
  llvm/test/CodeGen/NVPTX/param-vectorize-kernel.ll

Index: llvm/test/CodeGen/NVPTX/param-vectorize-kernel.ll
===
--- /dev/null
+++ llvm/test/CodeGen/NVPTX/param-vectorize-kernel.ll
@@ -0,0 +1,456 @@
+; RUN: llc < %s -mtriple=nvptx-unknown-unknown | FileCheck %s
+;
+; Check that parameters of a __global__ (kernel) function do not get increased
+; alignment, and no additional vectorization is performed on loads/stores with
+; that parameters.
+;
+; Test IR is a minimized version of IR generated with the following command
+; from the source code below:
+; $ clang++ -O3 --cuda-gpu-arch=sm_35 -S -emit-llvm src.cu
+;
+; 
+; #include 
+;
+; struct St4x1 { uint32_t field[1]; };
+; struct St4x2 { uint32_t field[2]; };
+; struct St4x3 { uint32_t field[3]; };
+; struct St4x4 { uint32_t field[4]; };
+; struct St4x5 { uint32_t field[5]; };
+; struct St4x6 { uint32_t field[6]; };
+; struct St4x7 { uint32_t field[7]; };
+; struct St4x8 { uint32_t field[8]; };
+; struct St8x1 { uint64_t field[1]; };
+; struct St8x2 { uint64_t field[2]; };
+; struct St8x3 { uint64_t field[3]; };
+; struct St8x4 { uint64_t field[4]; };
+;
+; #define DECLARE_FUNCTION(StName)\
+; static __global__  __attribute__((noinline))\
+; void foo_##StName(struct StName in, struct StName* ret) {   \
+;   const unsigned size = sizeof(ret->field) / sizeof(*ret->field);   \
+;   for (unsigned i = 0; i != size; ++i)  \
+; ret->field[i] = in.field[i];\
+; }   \
+;
+; DECLARE_FUNCTION(St4x1)
+; DECLARE_FUNCTION(St4x2)
+; DECLARE_FUNCTION(St4x3)
+; DECLARE_FUNCTION(St4x4)
+; DECLARE_FUNCTION(St4x5)
+; DECLARE_FUNCTION(St4x6)
+; DECLARE_FUNCTION(St4x7)
+; DECLARE_FUNCTION(St4x8)
+; DECLARE_FUNCTION(St8x1)
+; DECLARE_FUNCTION(St8x2)
+; DECLARE_FUNCTION(St8x3)
+; DECLARE_FUNCTION(St8x4)
+; 
+
+%struct.St4x1 = type { [1 x i32] }
+%struct.St4x2 = type { [2 x i32] }
+%struct.St4x3 = type { [3 x i32] }
+%struct.St4x4 = type { [4 x i32] }
+%struct.St4x5 = type { [5 x i32] }
+%struct.St4x6 = type { [6 x i32] }
+%struct.St4x7 = type { [7 x i32] }
+%struct.St4x8 = type { [8 x i32] }
+%struct.St8x1 = type { [1 x i64] }
+%struct.St8x2 = type { [2 x i64] }
+%struct.St8x3 = type { [3 x i64] }
+%struct.St8x4 = type { [4 x i64] }
+
+define dso_local void @foo_St4x1(%struct.St4x1* nocapture noundef readonly byval(%struct.St4x1) align 4 %in, %struct.St4x1* nocapture noundef writeonly %ret) {
+  ; CHECK-LABEL: .visible .func foo_St4x1(
+  ; CHECK:   .param .align 4 .b8 foo_St4x1_param_0[4],
+  ; CHECK:   .param .b32 foo_St4x1_param_1
+  ; CHECK:   )
+  ; CHECK:   ld.param.u32 [[R1:%r[0-9]+]], [foo_St4x1_param_1];
+  ; CHECK:   ld.param.u32 [[R2:%r[0-9]+]], [foo_St4x1_param_0];
+  ; CHECK:   st.u32  [[[R1]]], [[R2]];
+  ; CHECK:   ret;
+  %arrayidx = getelementptr inbounds %struct.St4x1, %struct.St4x1* %in, i64 0, i32 0, i64 0
+  %1 = load i32, i32* %arrayidx, align 4
+  %arrayidx3 = getelementptr inbounds %struct.St4x1, %struct.St4x1* %ret, i64 0, i32 0, i64 0
+  store i32 %1, i32* %arrayidx3, align 4
+  ret void
+}
+
+define dso_local void @foo_St4x2(%struct.St4x2* nocapture noundef readonly byval(%struct.St4x2) align 4 %in, %struct.St4x2* nocapture noundef writeonly %ret) {
+  ; CHECK-LABEL: .visible .func foo_St4x2(
+  ; CHECK:   .param .align 4 .b8 foo_St4x2_param_0[8],
+  ; CHECK:   .param .b32 foo_St4x2_param_1
+  ; CHECK:   )
+  ; CHECK:   ld.param.u32 [[R1:%r[0-9]+]], [foo_St4x2_param_1];
+  ; CHECK:   ld.param.u32 [[R2:%r[0-9]+]], [foo_St4x2_param_0];
+  ; CHECK:   st.u32  [[[R1]]], [[R2]];
+  ; CHECK:   ld.param.u32 [[R3:%r[0-9]+]], [foo_St4x2_param_0+4];
+  ; CHECK:   st.u32  [[[R1]]+4], [[R3]];
+  ; CHECK:   ret;
+  %arrayidx = getelementp

[PATCH] D120129: [NVPTX] Enhance vectorization of ld.param & st.param

[Daniil Kovalev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
kovdan01 marked 2 inline comments as done.
Closed by commit rG828b63c30943: [NVPTX] Enhance vectorization of ld.param 
& st.param (authored by kovdan01).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120129/new/

https://reviews.llvm.org/D120129

Files:
  clang/test/CodeGenCUDA/device-fun-linkage.cu
  llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
  llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
  llvm/lib/Target/NVPTX/NVPTXISelLowering.h
  llvm/lib/Target/NVPTX/NVPTXLowerArgs.cpp
  llvm/test/CodeGen/NVPTX/param-vectorize-device.ll
  llvm/test/CodeGen/NVPTX/param-vectorize-kernel.ll

Index: llvm/test/CodeGen/NVPTX/param-vectorize-kernel.ll
===
--- /dev/null
+++ llvm/test/CodeGen/NVPTX/param-vectorize-kernel.ll
@@ -0,0 +1,456 @@
+; RUN: llc < %s -mtriple=nvptx-unknown-unknown | FileCheck %s
+;
+; Check that parameters of a __global__ (kernel) function do not get increased
+; alignment, and no additional vectorization is performed on loads/stores with
+; that parameters.
+;
+; Test IR is a minimized version of IR generated with the following command
+; from the source code below:
+; $ clang++ -O3 --cuda-gpu-arch=sm_35 -S -emit-llvm src.cu
+;
+; 
+; #include 
+;
+; struct St4x1 { uint32_t field[1]; };
+; struct St4x2 { uint32_t field[2]; };
+; struct St4x3 { uint32_t field[3]; };
+; struct St4x4 { uint32_t field[4]; };
+; struct St4x5 { uint32_t field[5]; };
+; struct St4x6 { uint32_t field[6]; };
+; struct St4x7 { uint32_t field[7]; };
+; struct St4x8 { uint32_t field[8]; };
+; struct St8x1 { uint64_t field[1]; };
+; struct St8x2 { uint64_t field[2]; };
+; struct St8x3 { uint64_t field[3]; };
+; struct St8x4 { uint64_t field[4]; };
+;
+; #define DECLARE_FUNCTION(StName)\
+; static __global__  __attribute__((noinline))\
+; void foo_##StName(struct StName in, struct StName* ret) {   \
+;   const unsigned size = sizeof(ret->field) / sizeof(*ret->field);   \
+;   for (unsigned i = 0; i != size; ++i)  \
+; ret->field[i] = in.field[i];\
+; }   \
+;
+; DECLARE_FUNCTION(St4x1)
+; DECLARE_FUNCTION(St4x2)
+; DECLARE_FUNCTION(St4x3)
+; DECLARE_FUNCTION(St4x4)
+; DECLARE_FUNCTION(St4x5)
+; DECLARE_FUNCTION(St4x6)
+; DECLARE_FUNCTION(St4x7)
+; DECLARE_FUNCTION(St4x8)
+; DECLARE_FUNCTION(St8x1)
+; DECLARE_FUNCTION(St8x2)
+; DECLARE_FUNCTION(St8x3)
+; DECLARE_FUNCTION(St8x4)
+; 
+
+%struct.St4x1 = type { [1 x i32] }
+%struct.St4x2 = type { [2 x i32] }
+%struct.St4x3 = type { [3 x i32] }
+%struct.St4x4 = type { [4 x i32] }
+%struct.St4x5 = type { [5 x i32] }
+%struct.St4x6 = type { [6 x i32] }
+%struct.St4x7 = type { [7 x i32] }
+%struct.St4x8 = type { [8 x i32] }
+%struct.St8x1 = type { [1 x i64] }
+%struct.St8x2 = type { [2 x i64] }
+%struct.St8x3 = type { [3 x i64] }
+%struct.St8x4 = type { [4 x i64] }
+
+define dso_local void @foo_St4x1(%struct.St4x1* nocapture noundef readonly byval(%struct.St4x1) align 4 %in, %struct.St4x1* nocapture noundef writeonly %ret) {
+  ; CHECK-LABEL: .visible .func foo_St4x1(
+  ; CHECK:   .param .align 4 .b8 foo_St4x1_param_0[4],
+  ; CHECK:   .param .b32 foo_St4x1_param_1
+  ; CHECK:   )
+  ; CHECK:   ld.param.u32 [[R1:%r[0-9]+]], [foo_St4x1_param_1];
+  ; CHECK:   ld.param.u32 [[R2:%r[0-9]+]], [foo_St4x1_param_0];
+  ; CHECK:   st.u32  [[[R1]]], [[R2]];
+  ; CHECK:   ret;
+  %arrayidx = getelementptr inbounds %struct.St4x1, %struct.St4x1* %in, i64 0, i32 0, i64 0
+  %1 = load i32, i32* %arrayidx, align 4
+  %arrayidx3 = getelementptr inbounds %struct.St4x1, %struct.St4x1* %ret, i64 0, i32 0, i64 0
+  store i32 %1, i32* %arrayidx3, align 4
+  ret void
+}
+
+define dso_local void @foo_St4x2(%struct.St4x2* nocapture noundef readonly byval(%struct.St4x2) align 4 %in, %struct.St4x2* nocapture noundef writeonly %ret) {
+  ; CHECK-LABEL: .visible .func foo_St4x2(
+  ; CHECK:   .param .align 4 .b8 foo_St4x2_param_0[8],
+  ; CHECK:   .param .b32 foo_St4x2_param_1
+  ; CHECK:   )
+  ; CHECK:   ld.param.u32 [[R1:%r[0-9]+]], [foo_St4x2_param_1];
+  ; CHECK:   ld.param.u32 [[R2:%r[0-9]+]], [foo_St4x2_param_0];
+  ; CHECK:   st.u32  [[[R1]]], [[R2]];
+  ; CHECK:   ld.param.u32 [[R3:%r[0-9]+]], [foo_St4x2_param_0+4];
+  ; CHECK:   st.u32  [[[R1]]+4], [[R3]];
+  ; CHECK:   ret;
+  %arrayidx = getelementptr inbounds %struct.St4x2, %struct.St4x2* %in, i64 0, i32 0, i6

[PATCH] D121549: [CodeGen] Define ABI breaking class members correctly

[Daniil Kovalev via Phabricator via cfe-commits]

kovdan01 updated this revision to Diff 417858.
kovdan01 added a comment.

Accidentally placed this revision ID in commit message related to other patch. 
Reverted that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121549/new/

https://reviews.llvm.org/D121549

Files:
  llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h
  llvm/include/llvm/CodeGen/MachineScheduler.h
  llvm/include/llvm/CodeGen/PBQP/ReductionRules.h
  llvm/include/llvm/CodeGen/RegAllocPBQP.h
  llvm/include/llvm/CodeGen/SelectionDAG.h
  llvm/lib/CodeGen/MachineScheduler.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp

Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
@@ -181,11 +181,11 @@
 /// clearGraphAttrs - Clear all previously defined node graph attributes.
 /// Intended to be used from a debugging tool (eg. gdb).
 void SelectionDAG::clearGraphAttrs() {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   NodeGraphAttrs.clear();
 #else
-  errs() << "SelectionDAG::clearGraphAttrs is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::clearGraphAttrs is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
 #endif
 }
 
@@ -193,11 +193,11 @@
 /// setGraphAttrs - Set graph attributes for a node. (eg. "color=red".)
 ///
 void SelectionDAG::setGraphAttrs(const SDNode *N, const char *Attrs) {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   NodeGraphAttrs[N] = Attrs;
 #else
-  errs() << "SelectionDAG::setGraphAttrs is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::setGraphAttrs is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
 #endif
 }
 
@@ -205,7 +205,7 @@
 /// getGraphAttrs - Get graph attributes for a node. (eg. "color=red".)
 /// Used from getNodeAttributes.
 std::string SelectionDAG::getGraphAttrs(const SDNode *N) const {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   std::map::const_iterator I =
 NodeGraphAttrs.find(N);
 
@@ -214,8 +214,8 @@
   else
 return "";
 #else
-  errs() << "SelectionDAG::getGraphAttrs is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::getGraphAttrs is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
   return std::string();
 #endif
 }
@@ -223,11 +223,11 @@
 /// setGraphColor - Convenience for setting node color attribute.
 ///
 void SelectionDAG::setGraphColor(const SDNode *N, const char *Color) {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   NodeGraphAttrs[N] = std::string("color=") + Color;
 #else
-  errs() << "SelectionDAG::setGraphColor is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::setGraphColor is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
 #endif
 }
 
Index: llvm/lib/CodeGen/MachineScheduler.cpp
===
--- llvm/lib/CodeGen/MachineScheduler.cpp
+++ llvm/lib/CodeGen/MachineScheduler.cpp
@@ -752,7 +752,7 @@
 }
 
 bool ScheduleDAGMI::checkSchedLimit() {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   if (NumInstrsScheduled == MISchedCutoff && MISchedCutoff != ~0U) {
 CurrentTop = CurrentBottom;
 return false;
@@ -2006,7 +2006,7 @@
   ReservedCycles.clear();
   ReservedCyclesIndex.clear();
   ResourceGroupSubUnitMasks.clear();
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   // Track the maximum number of stall cycles that could arise either from the
   // latency of a DAG edge or the number of cycles that a processor resource is
   // reserved (SchedBoundary::ReservedCycles).
@@ -2194,7 +2194,7 @@
   unsigned NRCycle, InstanceIdx;
   std::tie(NRCycle, InstanceIdx) = getNextResourceCycle(SC, ResIdx, Cycles);
   if (NRCycle > CurrCycle) {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
 MaxObservedStall = std::max(Cycles, MaxObservedStall);
 #endif
 LLVM_DEBUG(dbgs() << "  SU(" << SU->NodeNum << ") "
@@ -2261,7 +2261,7 @@
 unsigned Idx) {
   assert(SU->getInstr() && "Scheduled SUnit must have instr");
 
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   // ReadyCycle was been bumped up to the CurrCycle when this node was
   // scheduled, but CurrCycle may have been eagerly advanced immediately after
   // scheduling, so may now be greater than ReadyCycle.
Index: llvm/include/llvm/CodeGen/SelectionDAG.h
===
--- llvm/include/llvm/CodeGen/SelectionDAG.h
+++ 

[PATCH] D120129: [NVPTX] Enhance vectorization of ld.param & st.param

[Daniil Kovalev via Phabricator via cfe-commits]

kovdan01 added a comment.

In D120129#3408088 , @jdoerfert wrote:

> Godbold seems to not have included this:
>
>   $ cat test.cpp
>   int main(){return 0;}
>   $ clang++ -fopenmp -fopenmp-targets=nvptx64 -Xopenmp-target -march=sm_80 
> test.cpp
>   ... static bool llvm::isa_impl_cl llvm::Metadata *>::doit(const From *) [To = llvm::ConstantAsMetadata, From = 
> const llvm::Metadata *]: Assertion `Val && "isa<> used on a null pointer"' 
> failed.

Thanks for the reproducer! Is the problem specific for sm_80 or not? I am 
unable to reproduce the issue with sm_75 and lower.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120129/new/

https://reviews.llvm.org/D120129

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120129: [NVPTX] Enhance vectorization of ld.param & st.param

[Daniil Kovalev via Phabricator via cfe-commits]

kovdan01 added a comment.

In D120129#3408168 , @jdoerfert wrote:

> You don't need to run it. If you use this command line it doesn't crash?

Yes, I run the same command (with sm_75 instead of sm_80 because I don't have 
libomptarget-nvptx-sm_80.bc), and it doesn't crash.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120129/new/

https://reviews.llvm.org/D120129

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120129: [NVPTX] Enhance vectorization of ld.param & st.param

[Daniil Kovalev via Phabricator via cfe-commits]

kovdan01 added a comment.

In D120129#3410479 , @jdoerfert wrote:

> Please revert this commit. It breaks any code using the cuda.11.0.2 
> libdevice.bc file, the source of the `null` `nvvm.annotations` and the 
> annotations with 5 arguments. See below.
> Once a fix is in, and a test is added, we can reapply this.
>
>   $ opt -S .../cuda-11.0.2/nvvm/libdevice/libdevice.10.bc | tail
>   !nvvm.annotations = !{!1, !2, !1, !3, !3, !3, !3, !4, !4, !3}
>   
>   !0 = !{i32 1, i32 4}
>   !1 = !{null, !"align", i32 8}
>   !2 = !{null, !"align", i32 8, !"align", i32 65544, !"align", i32 131080}
>   !3 = !{null, !"align", i32 16}
>   !4 = !{null, !"align", i32 16, !"align", i32 65552, !"align", i32 131088}
>   !5 = distinct !{!5, !6}

Got it, will revert the patch shortly. Could you please describe what do you 
mean by "break" in a bit more detail? Is that only about poorly designed and 
failing assertions in `getFunctionParamOptimizedAlign` or there are some other 
(maybe even functional) issues? If that's only about assertions, won't using 
`isKernelFunction` (as suggested in D122550 ) 
help?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120129/new/

https://reviews.llvm.org/D120129

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120129: [NVPTX] Enhance vectorization of ld.param & st.param

[Daniil Kovalev via Phabricator via cfe-commits]

kovdan01 added a comment.

In D120129#3410510 , @jdoerfert wrote:

> The two assertions introduced here do not hold for the libdevice.bc above. So 
> whenever we link the above we will cause the assertions to fail. That's what 
> I mean with break. isKernelFunction is probably going to fix all that. For a 
> test, copy annotations like the above into one of the .ll files and make sure 
> it contains a private/internal function w/ arguments as well.

OK, thanks for the explanation! Can we just submit a new patch with a fix (like 
D122550 ) instead of reverting this one? The 
problem with revert is that we should also revert D122381 
 which depends on this patch. Also, adding 
tests for `null` `nvvm.annotations` and the annotations with 5 arguments IMHO 
will look better when submitted as a separate patch. So, it revert crucial for 
you or can we just submit a fix separately?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120129/new/

https://reviews.llvm.org/D120129

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120129: [NVPTX] Enhance vectorization of ld.param & st.param

[Daniil Kovalev via Phabricator via cfe-commits]

kovdan01 added a comment.

In D120129#3410514 , @jdoerfert wrote:

> Our internal build bots and CI for some projects are broken for 3 days. I 
> wish to unbreak them so we get actual meaningful results, e.g., see if 
> something else is breaking our build. I'm fine with a separate patch on top 
> but I would prefer it now so people can run code on Monday.

OK, I'll submit a separate patch within an hour, and if it looks good to you - 
will merge it immediately. The fix looks simple enough so there is no need to 
wait for additional approvals.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120129/new/

https://reviews.llvm.org/D120129

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121549: Define ABI breaking class members correctly

[Daniil Kovalev via Phabricator via cfe-commits]

kovdan01 created this revision.
kovdan01 added reviewers: dexonsmith, craig.topper.
Herald added subscribers: sdasgup3, wenzhicui, wrengr, Chia-hungDuan, dcaballe, 
cota, teijeong, rdzhabarov, tatianashp, ThomasRaoux, msifontes, ecnelises, 
jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, 
antiagainst, shauheen, mravishankar, rriddle, mehdi_amini, javed.absar, 
hiraditya, qcolombet, MatzeB.
Herald added a reviewer: antiagainst.
Herald added a project: All.
kovdan01 requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, stephenneuendorffer, 
nicolasvasilache.
Herald added projects: clang, MLIR, LLVM.

Non-static class members declared under #ifndef NDEBUG should be declared
under #if LLVM_ENABLE_ABI_BREAKING_CHECKS to make headers library-friendly and
allow cross-linking.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121549

Files:
  clang/lib/Sema/TypeLocBuilder.cpp
  clang/lib/Sema/TypeLocBuilder.h
  llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h
  llvm/include/llvm/CodeGen/MachineScheduler.h
  llvm/include/llvm/CodeGen/PBQP/ReductionRules.h
  llvm/include/llvm/CodeGen/RegAllocPBQP.h
  llvm/include/llvm/CodeGen/SelectionDAG.h
  llvm/lib/CodeGen/MachineScheduler.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
  llvm/lib/Target/SystemZ/SystemZHazardRecognizer.h
  mlir/lib/Target/SPIRV/Deserialization/Deserializer.h

Index: mlir/lib/Target/SPIRV/Deserialization/Deserializer.h
===
--- mlir/lib/Target/SPIRV/Deserialization/Deserializer.h
+++ mlir/lib/Target/SPIRV/Deserialization/Deserializer.h
@@ -598,6 +598,7 @@
   /// A list of all structs which have unresolved member types.
   SmallVector deferredStructTypesInfos;
 
+  // TODO: place logger undef #if LLVM_ENABLE_ABI_BREAKING_CHECKS
 #ifndef NDEBUG
   /// A logger used to emit information during the deserialzation process.
   llvm::ScopedPrinter logger;
Index: llvm/lib/Target/SystemZ/SystemZHazardRecognizer.h
===
--- llvm/lib/Target/SystemZ/SystemZHazardRecognizer.h
+++ llvm/lib/Target/SystemZ/SystemZHazardRecognizer.h
@@ -141,6 +141,7 @@
   /// a negative value means it would be good to schedule SU next.
   int resourcesCost(SUnit *SU);
 
+  // TODO: place CurGroupDbg undef #if LLVM_ENABLE_ABI_BREAKING_CHECKS
 #ifndef NDEBUG
   // Debug dumping.
   std::string CurGroupDbg; // current group as text
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
@@ -181,11 +181,11 @@
 /// clearGraphAttrs - Clear all previously defined node graph attributes.
 /// Intended to be used from a debugging tool (eg. gdb).
 void SelectionDAG::clearGraphAttrs() {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   NodeGraphAttrs.clear();
 #else
-  errs() << "SelectionDAG::clearGraphAttrs is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::setGraphAttrs is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
 #endif
 }
 
@@ -193,11 +193,11 @@
 /// setGraphAttrs - Set graph attributes for a node. (eg. "color=red".)
 ///
 void SelectionDAG::setGraphAttrs(const SDNode *N, const char *Attrs) {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   NodeGraphAttrs[N] = Attrs;
 #else
-  errs() << "SelectionDAG::setGraphAttrs is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::setGraphAttrs is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
 #endif
 }
 
@@ -205,7 +205,7 @@
 /// getGraphAttrs - Get graph attributes for a node. (eg. "color=red".)
 /// Used from getNodeAttributes.
 std::string SelectionDAG::getGraphAttrs(const SDNode *N) const {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   std::map::const_iterator I =
 NodeGraphAttrs.find(N);
 
@@ -214,8 +214,8 @@
   else
 return "";
 #else
-  errs() << "SelectionDAG::getGraphAttrs is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::getGraphAttrs is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
   return std::string();
 #endif
 }
@@ -223,11 +223,11 @@
 /// setGraphColor - Convenience for setting node color attribute.
 ///
 void SelectionDAG::setGraphColor(const SDNode *N, const char *Color) {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   NodeGraphAttrs[N] = std::string("color=") + Color;
 #else
-  errs() << "SelectionDAG::setGraphColor is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() <<

[PATCH] D121549: Define ABI breaking class members correctly

[Daniil Kovalev via Phabricator via cfe-commits]

kovdan01 updated this revision to Diff 414929.
kovdan01 added a comment.

Fix typo


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121549/new/

https://reviews.llvm.org/D121549

Files:
  clang/lib/Sema/TypeLocBuilder.cpp
  clang/lib/Sema/TypeLocBuilder.h
  llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h
  llvm/include/llvm/CodeGen/MachineScheduler.h
  llvm/include/llvm/CodeGen/PBQP/ReductionRules.h
  llvm/include/llvm/CodeGen/RegAllocPBQP.h
  llvm/include/llvm/CodeGen/SelectionDAG.h
  llvm/lib/CodeGen/MachineScheduler.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
  llvm/lib/Target/SystemZ/SystemZHazardRecognizer.h
  mlir/lib/Target/SPIRV/Deserialization/Deserializer.h

Index: mlir/lib/Target/SPIRV/Deserialization/Deserializer.h
===
--- mlir/lib/Target/SPIRV/Deserialization/Deserializer.h
+++ mlir/lib/Target/SPIRV/Deserialization/Deserializer.h
@@ -598,6 +598,7 @@
   /// A list of all structs which have unresolved member types.
   SmallVector deferredStructTypesInfos;
 
+  // TODO: place logger under #if LLVM_ENABLE_ABI_BREAKING_CHECKS
 #ifndef NDEBUG
   /// A logger used to emit information during the deserialzation process.
   llvm::ScopedPrinter logger;
Index: llvm/lib/Target/SystemZ/SystemZHazardRecognizer.h
===
--- llvm/lib/Target/SystemZ/SystemZHazardRecognizer.h
+++ llvm/lib/Target/SystemZ/SystemZHazardRecognizer.h
@@ -141,6 +141,7 @@
   /// a negative value means it would be good to schedule SU next.
   int resourcesCost(SUnit *SU);
 
+  // TODO: place CurGroupDbg under #if LLVM_ENABLE_ABI_BREAKING_CHECKS
 #ifndef NDEBUG
   // Debug dumping.
   std::string CurGroupDbg; // current group as text
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
@@ -181,11 +181,11 @@
 /// clearGraphAttrs - Clear all previously defined node graph attributes.
 /// Intended to be used from a debugging tool (eg. gdb).
 void SelectionDAG::clearGraphAttrs() {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   NodeGraphAttrs.clear();
 #else
-  errs() << "SelectionDAG::clearGraphAttrs is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::clearGraphAttrs is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
 #endif
 }
 
@@ -193,11 +193,11 @@
 /// setGraphAttrs - Set graph attributes for a node. (eg. "color=red".)
 ///
 void SelectionDAG::setGraphAttrs(const SDNode *N, const char *Attrs) {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   NodeGraphAttrs[N] = Attrs;
 #else
-  errs() << "SelectionDAG::setGraphAttrs is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::setGraphAttrs is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
 #endif
 }
 
@@ -205,7 +205,7 @@
 /// getGraphAttrs - Get graph attributes for a node. (eg. "color=red".)
 /// Used from getNodeAttributes.
 std::string SelectionDAG::getGraphAttrs(const SDNode *N) const {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   std::map::const_iterator I =
 NodeGraphAttrs.find(N);
 
@@ -214,8 +214,8 @@
   else
 return "";
 #else
-  errs() << "SelectionDAG::getGraphAttrs is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::getGraphAttrs is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
   return std::string();
 #endif
 }
@@ -223,11 +223,11 @@
 /// setGraphColor - Convenience for setting node color attribute.
 ///
 void SelectionDAG::setGraphColor(const SDNode *N, const char *Color) {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   NodeGraphAttrs[N] = std::string("color=") + Color;
 #else
-  errs() << "SelectionDAG::setGraphColor is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::setGraphColor is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
 #endif
 }
 
Index: llvm/lib/CodeGen/MachineScheduler.cpp
===
--- llvm/lib/CodeGen/MachineScheduler.cpp
+++ llvm/lib/CodeGen/MachineScheduler.cpp
@@ -752,7 +752,7 @@
 }
 
 bool ScheduleDAGMI::checkSchedLimit() {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   if (NumInstrsScheduled == MISchedCutoff && MISchedCutoff != ~0U) {
 CurrentTop = CurrentBottom;
 return false;
@@ -2006,7 +2006,7 @@
   ReservedCycles.clear();
   ReservedCyclesIndex.clear();
   ResourceGroupSubUnitMasks.clear();
-#ifndef NDEBUG
+#i

[PATCH] D121549: Define ABI breaking class members correctly

[Daniil Kovalev via Phabricator via cfe-commits]

kovdan01 updated this revision to Diff 415022.
kovdan01 added a comment.

Keep private headers untouched


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121549/new/

https://reviews.llvm.org/D121549

Files:
  llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h
  llvm/include/llvm/CodeGen/MachineScheduler.h
  llvm/include/llvm/CodeGen/PBQP/ReductionRules.h
  llvm/include/llvm/CodeGen/RegAllocPBQP.h
  llvm/include/llvm/CodeGen/SelectionDAG.h
  llvm/lib/CodeGen/MachineScheduler.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp

Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
@@ -181,11 +181,11 @@
 /// clearGraphAttrs - Clear all previously defined node graph attributes.
 /// Intended to be used from a debugging tool (eg. gdb).
 void SelectionDAG::clearGraphAttrs() {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   NodeGraphAttrs.clear();
 #else
-  errs() << "SelectionDAG::clearGraphAttrs is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::clearGraphAttrs is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
 #endif
 }
 
@@ -193,11 +193,11 @@
 /// setGraphAttrs - Set graph attributes for a node. (eg. "color=red".)
 ///
 void SelectionDAG::setGraphAttrs(const SDNode *N, const char *Attrs) {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   NodeGraphAttrs[N] = Attrs;
 #else
-  errs() << "SelectionDAG::setGraphAttrs is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::setGraphAttrs is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
 #endif
 }
 
@@ -205,7 +205,7 @@
 /// getGraphAttrs - Get graph attributes for a node. (eg. "color=red".)
 /// Used from getNodeAttributes.
 std::string SelectionDAG::getGraphAttrs(const SDNode *N) const {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   std::map::const_iterator I =
 NodeGraphAttrs.find(N);
 
@@ -214,8 +214,8 @@
   else
 return "";
 #else
-  errs() << "SelectionDAG::getGraphAttrs is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::getGraphAttrs is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
   return std::string();
 #endif
 }
@@ -223,11 +223,11 @@
 /// setGraphColor - Convenience for setting node color attribute.
 ///
 void SelectionDAG::setGraphColor(const SDNode *N, const char *Color) {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   NodeGraphAttrs[N] = std::string("color=") + Color;
 #else
-  errs() << "SelectionDAG::setGraphColor is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::setGraphColor is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
 #endif
 }
 
Index: llvm/lib/CodeGen/MachineScheduler.cpp
===
--- llvm/lib/CodeGen/MachineScheduler.cpp
+++ llvm/lib/CodeGen/MachineScheduler.cpp
@@ -752,7 +752,7 @@
 }
 
 bool ScheduleDAGMI::checkSchedLimit() {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   if (NumInstrsScheduled == MISchedCutoff && MISchedCutoff != ~0U) {
 CurrentTop = CurrentBottom;
 return false;
@@ -2006,7 +2006,7 @@
   ReservedCycles.clear();
   ReservedCyclesIndex.clear();
   ResourceGroupSubUnitMasks.clear();
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   // Track the maximum number of stall cycles that could arise either from the
   // latency of a DAG edge or the number of cycles that a processor resource is
   // reserved (SchedBoundary::ReservedCycles).
@@ -2194,7 +2194,7 @@
   unsigned NRCycle, InstanceIdx;
   std::tie(NRCycle, InstanceIdx) = getNextResourceCycle(SC, ResIdx, Cycles);
   if (NRCycle > CurrCycle) {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
 MaxObservedStall = std::max(Cycles, MaxObservedStall);
 #endif
 LLVM_DEBUG(dbgs() << "  SU(" << SU->NodeNum << ") "
@@ -2261,7 +2261,7 @@
 unsigned Idx) {
   assert(SU->getInstr() && "Scheduled SUnit must have instr");
 
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   // ReadyCycle was been bumped up to the CurrCycle when this node was
   // scheduled, but CurrCycle may have been eagerly advanced immediately after
   // scheduling, so may now be greater than ReadyCycle.
Index: llvm/include/llvm/CodeGen/SelectionDAG.h
===
--- llvm/include/llvm/CodeGen/SelectionDAG.h
+++ llvm/include/llvm/CodeGen/SelectionDAG.h
@@ -467,7 +467,7 @@
   

[PATCH] D121549: Define ABI breaking class members correctly

[Daniil Kovalev via Phabricator via cfe-commits]

kovdan01 marked an inline comment as done.
kovdan01 added inline comments.



Comment at: mlir/lib/Target/SPIRV/Deserialization/Deserializer.h:601
 
+  // TODO: place logger under #if LLVM_ENABLE_ABI_BREAKING_CHECKS
 #ifndef NDEBUG

mehdi_amini wrote:
> This is a private header (you're in the `lib` directory and not in the 
> `include` directory)
> 
> Same for the SystemZ one above I believe.
Makes sense, fixed, thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121549/new/

https://reviews.llvm.org/D121549

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121549: Define ABI breaking class members correctly

[Daniil Kovalev via Phabricator via cfe-commits]

kovdan01 updated this revision to Diff 415049.
kovdan01 marked an inline comment as done.
kovdan01 removed projects: MLIR, clang.
kovdan01 added a comment.

Fix formatting problems found on pre-merge checks


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121549/new/

https://reviews.llvm.org/D121549

Files:
  llvm/include/llvm/CodeGen/GlobalISel/GISelWorkList.h
  llvm/include/llvm/CodeGen/MachineScheduler.h
  llvm/include/llvm/CodeGen/PBQP/ReductionRules.h
  llvm/include/llvm/CodeGen/RegAllocPBQP.h
  llvm/include/llvm/CodeGen/SelectionDAG.h
  llvm/lib/CodeGen/MachineScheduler.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp

Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
@@ -181,11 +181,11 @@
 /// clearGraphAttrs - Clear all previously defined node graph attributes.
 /// Intended to be used from a debugging tool (eg. gdb).
 void SelectionDAG::clearGraphAttrs() {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   NodeGraphAttrs.clear();
 #else
-  errs() << "SelectionDAG::clearGraphAttrs is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::clearGraphAttrs is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
 #endif
 }
 
@@ -193,11 +193,11 @@
 /// setGraphAttrs - Set graph attributes for a node. (eg. "color=red".)
 ///
 void SelectionDAG::setGraphAttrs(const SDNode *N, const char *Attrs) {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   NodeGraphAttrs[N] = Attrs;
 #else
-  errs() << "SelectionDAG::setGraphAttrs is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::setGraphAttrs is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
 #endif
 }
 
@@ -205,7 +205,7 @@
 /// getGraphAttrs - Get graph attributes for a node. (eg. "color=red".)
 /// Used from getNodeAttributes.
 std::string SelectionDAG::getGraphAttrs(const SDNode *N) const {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   std::map::const_iterator I =
 NodeGraphAttrs.find(N);
 
@@ -214,8 +214,8 @@
   else
 return "";
 #else
-  errs() << "SelectionDAG::getGraphAttrs is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::getGraphAttrs is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
   return std::string();
 #endif
 }
@@ -223,11 +223,11 @@
 /// setGraphColor - Convenience for setting node color attribute.
 ///
 void SelectionDAG::setGraphColor(const SDNode *N, const char *Color) {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   NodeGraphAttrs[N] = std::string("color=") + Color;
 #else
-  errs() << "SelectionDAG::setGraphColor is only available in debug builds"
- << " on systems with Graphviz or gv!\n";
+  errs() << "SelectionDAG::setGraphColor is only available in builds with "
+ << "ABI breaking checks enabled on systems with Graphviz or gv!\n";
 #endif
 }
 
Index: llvm/lib/CodeGen/MachineScheduler.cpp
===
--- llvm/lib/CodeGen/MachineScheduler.cpp
+++ llvm/lib/CodeGen/MachineScheduler.cpp
@@ -752,7 +752,7 @@
 }
 
 bool ScheduleDAGMI::checkSchedLimit() {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   if (NumInstrsScheduled == MISchedCutoff && MISchedCutoff != ~0U) {
 CurrentTop = CurrentBottom;
 return false;
@@ -2006,7 +2006,7 @@
   ReservedCycles.clear();
   ReservedCyclesIndex.clear();
   ResourceGroupSubUnitMasks.clear();
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   // Track the maximum number of stall cycles that could arise either from the
   // latency of a DAG edge or the number of cycles that a processor resource is
   // reserved (SchedBoundary::ReservedCycles).
@@ -2194,7 +2194,7 @@
   unsigned NRCycle, InstanceIdx;
   std::tie(NRCycle, InstanceIdx) = getNextResourceCycle(SC, ResIdx, Cycles);
   if (NRCycle > CurrCycle) {
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
 MaxObservedStall = std::max(Cycles, MaxObservedStall);
 #endif
 LLVM_DEBUG(dbgs() << "  SU(" << SU->NodeNum << ") "
@@ -2261,7 +2261,7 @@
 unsigned Idx) {
   assert(SU->getInstr() && "Scheduled SUnit must have instr");
 
-#ifndef NDEBUG
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS
   // ReadyCycle was been bumped up to the CurrCycle when this node was
   // scheduled, but CurrCycle may have been eagerly advanced immediately after
   // scheduling, so may now be greater than ReadyCycle.
Index: llvm/include/llvm/CodeGen/SelectionDAG.h
===
--- llvm/in