[llvm] beb5213 - [MCA][InstrBuilder] Check for the presence of flag VariadicOpsAreDefs.

[Andrea Di Biagio via llvm-commits]

Author: Andrea Di Biagio
Date: 2021-06-15T09:52:38+01:00
New Revision: beb5213a2ee56bbf554ccdd1908c48d10e244dab

URL: https://github.com/llvm/llvm-project/commit/beb5213a2ee56bbf554ccdd1908c48d10e244dab
DIFF: https://github.com/llvm/llvm-project/commit/beb5213a2ee56bbf554ccdd1908c48d10e244dab.diff

LOG: [MCA][InstrBuilder] Check for the presence of flag VariadicOpsAreDefs.

This patch fixes the logic that checks for variadic register definitions,

Before llvm-svn 348114 (commit 4cf35b4ab0b), it was not possible to explicitly
mark variadic operands as definitions. By default, variadic operands of an
MCInst were always assumed to be uses. A number of had-hoc checks were
introduced in the InstrBuilder to fix the processing of variadic register
operands of ARM ldm/stm variants.

This patch simply replaces those old (and buggy) checks with a much simpler (and
correct) check for MCID::Flag::VariadicOpsAreDefs.

Added: 
    

Modified: 
    llvm/lib/MCA/InstrBuilder.cpp
    llvm/test/tools/llvm-mca/ARM/cortex-a57-carryover.s

Removed: 
    


################################################################################
diff  --git a/llvm/lib/MCA/InstrBuilder.cpp b/llvm/lib/MCA/InstrBuilder.cpp
index f79dca60d612b..fa11beb711ef9 100644
--- a/llvm/lib/MCA/InstrBuilder.cpp
+++ b/llvm/lib/MCA/InstrBuilder.cpp
@@ -391,15 +391,7 @@ void InstrBuilder::populateWrites(InstrDesc &ID, const MCInst &MCI,
   if (!NumVariadicOps)
     return;
 
-  // FIXME: if an instruction opcode is flagged 'mayStore', and it has no
-  // "unmodeledSideEffects', then this logic optimistically assumes that any
-  // extra register operands in the variadic sequence is not a register
-  // definition.
-  //
-  // Otherwise, we conservatively assume that any register operand from the
-  // variadic sequence is both a register read and a register write.
-  bool AssumeUsesOnly = MCDesc.mayStore() && !MCDesc.mayLoad() &&
-                        !MCDesc.hasUnmodeledSideEffects();
+  bool AssumeUsesOnly = !MCDesc.variadicOpsAreDefs();
   CurrentDef = NumExplicitDefs + NumImplicitDefs + MCDesc.hasOptionalDef();
   for (unsigned I = 0, OpIndex = MCDesc.getNumOperands();
        I < NumVariadicOps && !AssumeUsesOnly; ++I, ++OpIndex) {
@@ -466,12 +458,7 @@ void InstrBuilder::populateReads(InstrDesc &ID, const MCInst &MCI,
 
   CurrentUse += NumImplicitUses;
 
-  // FIXME: If an instruction opcode is marked as 'mayLoad', and it has no
-  // "unmodeledSideEffects", then this logic optimistically assumes that any
-  // extra register operand in the variadic sequence is not a register
-  // definition.
-  bool AssumeDefsOnly = !MCDesc.mayStore() && MCDesc.mayLoad() &&
-                        !MCDesc.hasUnmodeledSideEffects();
+  bool AssumeDefsOnly = MCDesc.variadicOpsAreDefs();
   for (unsigned I = 0, OpIndex = MCDesc.getNumOperands();
        I < NumVariadicOps && !AssumeDefsOnly; ++I, ++OpIndex) {
     const MCOperand &Op = MCI.getOperand(OpIndex);

diff  --git a/llvm/test/tools/llvm-mca/ARM/cortex-a57-carryover.s b/llvm/test/tools/llvm-mca/ARM/cortex-a57-carryover.s
index c058c028cbb69..ec99d704f0e5e 100644
--- a/llvm/test/tools/llvm-mca/ARM/cortex-a57-carryover.s
+++ b/llvm/test/tools/llvm-mca/ARM/cortex-a57-carryover.s
@@ -80,6 +80,6 @@
 # CHECK-NEXT: [3]: Average time elapsed from WB until retire stage
 
 # CHECK:            [0]    [1]    [2]    [3]
-# CHECK-NEXT: 0.     10    5.5    0.1    0.0       pop	{r3, r4, r5, r6, r7, pc}
+# CHECK-NEXT: 0.     10    5.5    2.7    0.0       pop	{r3, r4, r5, r6, r7, pc}
 # CHECK-NEXT: 1.     10    3.6    0.0    3.9       nop
-# CHECK-NEXT:        10    4.6    0.1    2.0       <total>
+# CHECK-NEXT:        10    4.6    1.4    2.0       <total>