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