<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<font size="-1">Hi, Andrea.<br>
<br>
Do you object to the comments, to the argument in them or to the
patch?<br>
<br>
Thank you,<br>
</font>
<pre class="moz-signature" cols="72">--
Evandro Menezes
</pre>
<div class="moz-cite-prefix">On 11/23/18 16:22, Andrea Di Biagio
wrote:<br>
</div>
<blockquote
cite="mid:CAMw=4mKU=aaPh4jQsW6OXTus1HgDc9RYc_TmgXXHDOJZb2qwgA@mail.gmail.com"
type="cite">
<meta http-equiv="Context-Type" content="text/html; charset=UTF-8">
<div><br>
</div>
<div><br>
<div class="gmail_quote">
<div dir="ltr">On Fri, 23 Nov 2018 at 21:20, Evandro Menezes
via llvm-commits <<a moz-do-not-send="true"
href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote">Author: evandro<br>
Date: Fri Nov 23 13:17:33 2018<br>
New Revision: 347504<br>
<br>
URL: <a moz-do-not-send="true"
href="http://llvm.org/viewvc/llvm-project?rev=347504&view=rev"
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=347504&view=rev</a><br>
Log:<br>
[TableGen] Emit more variant transitions<br>
<br>
...<br>
Preferably, `llvm-mca` should instead assume a reasonable
default when a<br>
variant transition is not based on `MCSchedPredicate` for a
given processor.<br>
This issue should be revisited in the future.</blockquote>
<div dir="auto"><br>
</div>
<div dir="auto">That’s not possible.</div>
<div dir="auto">llvm-mca should never guess a scheduling
class. That is bad design and Is to avoid as much as
possible.</div>
<div dir="auto"><br>
</div>
<div dir="auto">As I wrote before: targets that care about
supporting llvm-mca should just use MCSchedPredicate.</div>
<div dir="auto">Any attempt to workaround the absence of
MCSchedPredicates is not an option for me.</div>
<div dir="auto"><br>
</div>
<blockquote class="gmail_quote"><br>
<br>
Differential revision: <a moz-do-not-send="true"
href="https://reviews.llvm.org/D54648" rel="noreferrer"
target="_blank">https://reviews.llvm.org/D54648</a><br>
<br>
Modified:<br>
llvm/trunk/include/llvm/Target/TargetSchedule.td<br>
llvm/trunk/test/tools/llvm-mca/AArch64/CortexA57/shifted-register.s<br>
llvm/trunk/test/tools/llvm-mca/AArch64/Cyclone/register-offset.s<br>
llvm/trunk/test/tools/llvm-mca/ARM/unsupported-write-variant.s<br>
llvm/trunk/utils/TableGen/SubtargetEmitter.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/Target/TargetSchedule.td<br>
URL: <a moz-do-not-send="true"
href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetSchedule.td?rev=347504&r1=347503&r2=347504&view=diff"
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetSchedule.td?rev=347504&r1=347503&r2=347504&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/Target/TargetSchedule.td
(original)<br>
+++ llvm/trunk/include/llvm/Target/TargetSchedule.td Fri Nov
23 13:17:33 2018<br>
@@ -373,6 +373,10 @@ class SchedPredicate<code pred> :
SchedP<br>
SchedMachineModel SchedModel = ?;<br>
code Predicate = pred;<br>
}<br>
+<br>
+// Define a predicate to be typically used as the default
case in a<br>
+// SchedVariant. It the SchedVariant does not use any
other predicate based on<br>
+// MCSchedPredicate, this is the default scheduling case
used by llvm-mca.<br>
def NoSchedPred : MCSchedPredicate<TruePred>;<br>
<br>
// Associate a predicate with a list of SchedReadWrites. By
default,<br>
<br>
Modified:
llvm/trunk/test/tools/llvm-mca/AArch64/CortexA57/shifted-register.s<br>
URL: <a moz-do-not-send="true"
href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-mca/AArch64/CortexA57/shifted-register.s?rev=347504&r1=347503&r2=347504&view=diff"
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-mca/AArch64/CortexA57/shifted-register.s?rev=347504&r1=347503&r2=347504&view=diff</a><br>
==============================================================================<br>
---
llvm/trunk/test/tools/llvm-mca/AArch64/CortexA57/shifted-register.s
(original)<br>
+++
llvm/trunk/test/tools/llvm-mca/AArch64/CortexA57/shifted-register.s
Fri Nov 23 13:17:33 2018<br>
@@ -1,7 +1,25 @@<br>
-# RUN: not llvm-mca -march=aarch64 -mcpu=cortex-a57
-resource-pressure=false < %s 2> %t<br>
-# RUN: FileCheck --input-file %t %s<br>
+# NOTE: Assertions have been autogenerated by
utils/update_mca_test_checks.py<br>
+# RUN: llvm-mca -march=aarch64 -mcpu=cortex-a57
-resource-pressure=false < %s | FileCheck %s<br>
<br>
add x0, x1, x2, lsl #3<br>
<br>
-# CHECK: error<br>
-# CHECK-SAME: unable to resolve scheduling class for write
variant.<br>
+# CHECK: Iterations: 100<br>
+# CHECK-NEXT: Instructions: 100<br>
+# CHECK-NEXT: Total Cycles: 53<br>
+# CHECK-NEXT: Total uOps: 100<br>
+<br>
+# CHECK: Dispatch Width: 3<br>
+# CHECK-NEXT: uOps Per Cycle: 1.89<br>
+# CHECK-NEXT: IPC: 1.89<br>
+# CHECK-NEXT: Block RThroughput: 0.5<br>
+<br>
+# CHECK: Instruction Info:<br>
+# CHECK-NEXT: [1]: #uOps<br>
+# CHECK-NEXT: [2]: Latency<br>
+# CHECK-NEXT: [3]: RThroughput<br>
+# CHECK-NEXT: [4]: MayLoad<br>
+# CHECK-NEXT: [5]: MayStore<br>
+# CHECK-NEXT: [6]: HasSideEffects (U)<br>
+<br>
+# CHECK: [1] [2] [3] [4] [5] [6]
Instructions:<br>
+# CHECK-NEXT: 1 1 0.50
add x0, x1, x2, lsl #3<br>
<br>
Modified:
llvm/trunk/test/tools/llvm-mca/AArch64/Cyclone/register-offset.s<br>
URL: <a moz-do-not-send="true"
href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-mca/AArch64/Cyclone/register-offset.s?rev=347504&r1=347503&r2=347504&view=diff"
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-mca/AArch64/Cyclone/register-offset.s?rev=347504&r1=347503&r2=347504&view=diff</a><br>
==============================================================================<br>
---
llvm/trunk/test/tools/llvm-mca/AArch64/Cyclone/register-offset.s
(original)<br>
+++
llvm/trunk/test/tools/llvm-mca/AArch64/Cyclone/register-offset.s
Fri Nov 23 13:17:33 2018<br>
@@ -1,9 +1,29 @@<br>
-# RUN: not llvm-mca -march=aarch64 -mcpu=cyclone
-resource-pressure=false < %s 2> %t<br>
-# RUN: FileCheck --input-file %t %s<br>
+# NOTE: Assertions have been autogenerated by
utils/update_mca_test_checks.py<br>
+# RUN: llvm-mca -march=aarch64 -mcpu=cyclone
-resource-pressure=false < %s | FileCheck %s<br>
<br>
ldr x7, [x1, #8]<br>
ldr x6, [x1, x2]<br>
ldr x4, [x1, x2, sxtx]<br>
<br>
-# CHECK: error<br>
-# CHECK-SAME: unable to resolve scheduling class for write
variant.<br>
+# CHECK: Iterations: 100<br>
+# CHECK-NEXT: Instructions: 300<br>
+# CHECK-NEXT: Total Cycles: 156<br>
+# CHECK-NEXT: Total uOps: 300<br>
+<br>
+# CHECK: Dispatch Width: 6<br>
+# CHECK-NEXT: uOps Per Cycle: 1.92<br>
+# CHECK-NEXT: IPC: 1.92<br>
+# CHECK-NEXT: Block RThroughput: 1.5<br>
+<br>
+# CHECK: Instruction Info:<br>
+# CHECK-NEXT: [1]: #uOps<br>
+# CHECK-NEXT: [2]: Latency<br>
+# CHECK-NEXT: [3]: RThroughput<br>
+# CHECK-NEXT: [4]: MayLoad<br>
+# CHECK-NEXT: [5]: MayStore<br>
+# CHECK-NEXT: [6]: HasSideEffects (U)<br>
+<br>
+# CHECK: [1] [2] [3] [4] [5] [6]
Instructions:<br>
+# CHECK-NEXT: 1 4 0.50 *
ldr x7, [x1, #8]<br>
+# CHECK-NEXT: 1 4 0.50 *
ldr x6, [x1, x2]<br>
+# CHECK-NEXT: 1 4 0.50 *
ldr x4, [x1, x2, sxtx]<br>
<br>
Modified:
llvm/trunk/test/tools/llvm-mca/ARM/unsupported-write-variant.s<br>
URL: <a moz-do-not-send="true"
href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-mca/ARM/unsupported-write-variant.s?rev=347504&r1=347503&r2=347504&view=diff"
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-mca/ARM/unsupported-write-variant.s?rev=347504&r1=347503&r2=347504&view=diff</a><br>
==============================================================================<br>
---
llvm/trunk/test/tools/llvm-mca/ARM/unsupported-write-variant.s
(original)<br>
+++
llvm/trunk/test/tools/llvm-mca/ARM/unsupported-write-variant.s
Fri Nov 23 13:17:33 2018<br>
@@ -1,4 +1,6 @@<br>
# RUN: not llvm-mca -march=arm -mcpu=swift -all-views=false
2>&1 < %s | FileCheck %s<br>
+# D54648 results in this test to become valid.<br>
+# XFAIL: *<br>
<br>
add r3, r1, r12, lsl #2<br>
<br>
<br>
Modified: llvm/trunk/utils/TableGen/SubtargetEmitter.cpp<br>
URL: <a moz-do-not-send="true"
href="http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/SubtargetEmitter.cpp?rev=347504&r1=347503&r2=347504&view=diff"
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/SubtargetEmitter.cpp?rev=347504&r1=347503&r2=347504&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/utils/TableGen/SubtargetEmitter.cpp
(original)<br>
+++ llvm/trunk/utils/TableGen/SubtargetEmitter.cpp Fri Nov
23 13:17:33 2018<br>
@@ -1504,9 +1504,9 @@ void collectVariantClasses(const
CodeGen<br>
continue;<br>
<br>
if (OnlyExpandMCInstPredicates) {<br>
- // Ignore this variant scheduling class if
transitions don't uses any<br>
+ // Ignore this variant scheduling class no
transitions use any meaningful<br>
// MCSchedPredicate definitions.<br>
- if (!all_of(SC.Transitions, [](const
CodeGenSchedTransition &T) {<br>
+ if (!any_of(SC.Transitions, [](const
CodeGenSchedTransition &T) {<br>
return hasMCSchedPredicates(T);<br>
}))<br>
continue;<br>
@@ -1560,6 +1560,7 @@ void
SubtargetEmitter::emitSchedModelHel<br>
PE.setExpandForMC(OnlyExpandMCInstPredicates);<br>
for (unsigned PI : ProcIndices) {<br>
OS << " ";<br>
+<br>
// Emit a guard on the processor ID.<br>
if (PI != 0) {<br>
OS << (OnlyExpandMCInstPredicates<br>
@@ -1573,11 +1574,23 @@ void
SubtargetEmitter::emitSchedModelHel<br>
for (const CodeGenSchedTransition &T :
SC.Transitions) {<br>
if (PI != 0 && !count(T.ProcIndices, PI))<br>
continue;<br>
+<br>
+ // Emit only transitions based on MCSchedPredicate,
if it's the case.<br>
+ // At least the transition specified by NoSchedPred
is emitted,<br>
+ // which becomes the default transition for those
variants otherwise<br>
+ // not based on MCSchedPredicate.<br>
+ // FIXME: preferably, llvm-mca should instead
assume a reasonable<br>
+ // default when a variant transition is not based
on MCSchedPredicate<br>
+ // for a given processor.</blockquote>
<div dir="auto"><br>
</div>
<div dir="auto">It is not going to change.</div>
<div dir="auto">It is intentionally done so that people start
adopting the new predicates.</div>
<div dir="auto">In my opinion that is not an option.</div>
<div dir="auto"><br>
</div>
<blockquote class="gmail_quote"><br>
+ if (OnlyExpandMCInstPredicates &&
!hasMCSchedPredicates(T))<br>
+ continue;<br>
+<br>
PE.setIndentLevel(3);<br>
emitPredicates(T,
SchedModels.getSchedClass(T.ToClassIdx), PE, OS);<br>
}<br>
<br>
OS << " }\n";<br>
+<br>
if (PI == 0)<br>
break;<br>
}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a moz-do-not-send="true"
href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a moz-do-not-send="true"
href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits"
rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote>
</div>
</div>
</blockquote>
<br>
</body>
</html>