Skip to content

Commit 58991ba

Browse files
committed
[ARM] Mark MVE loads/store as not having side effects
The hasSideEffect parameter is usually automatically inferred from instruction patterns. For some of our MVE instructions, we do not have patterns though, such as for the pre/post inc loads and stores. This instead specifies the flag manually on the base MVE_VLDRSTR_base tablegen class, making sure we get this correct. This can help with scheduling multiple loads more optimally. Here I've added a unittest as a more direct form of testing. Differential Revision: https://reviews.llvm.org/D73117
1 parent cd47071 commit 58991ba

File tree

3 files changed

+89
-1
lines changed

3 files changed

+89
-1
lines changed

llvm/lib/Target/ARM/ARMInstrMVE.td

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4974,6 +4974,7 @@ class MVE_vldst24_base<bit writeback, bit fourregs, bits<2> stage, bits<2> size,
49744974

49754975
let mayLoad = load;
49764976
let mayStore = !eq(load,0);
4977+
let hasSideEffects = 0;
49774978
}
49784979

49794980
// A parameter class used to encapsulate all the ways the writeback
@@ -5169,6 +5170,7 @@ class MVE_VLDRSTR_base<MVE_ldst_direction dir, bit U, bit P, bit W, bit opc,
51695170

51705171
let mayLoad = dir.load;
51715172
let mayStore = !eq(dir.load,0);
5173+
let hasSideEffects = 0;
51725174
let validForTailPredication = 1;
51735175
}
51745176

llvm/test/CodeGen/Thumb2/LowOverheadLoops/mve-float-loops.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1357,9 +1357,9 @@ define arm_aapcs_vfpcc void @half_short_mul(half* nocapture readonly %a, i16* no
13571357
; CHECK-NEXT: dls lr, lr
13581358
; CHECK-NEXT: .LBB8_4: @ %vector.body
13591359
; CHECK-NEXT: @ =>This Inner Loop Header: Depth=1
1360+
; CHECK-NEXT: vldrh.u32 q0, [r5], #8
13601361
; CHECK-NEXT: ldr.w r9, [r4]
13611362
; CHECK-NEXT: ldr.w r8, [r4, #4]
1362-
; CHECK-NEXT: vldrh.u32 q0, [r5], #8
13631363
; CHECK-NEXT: adds r4, #8
13641364
; CHECK-NEXT: vmov r7, s0
13651365
; CHECK-NEXT: vmov.16 q1[0], r7

llvm/unittests/Target/ARM/MachineInstrTest.cpp

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -525,3 +525,89 @@ TEST(MachineInstrValidTailPredication, IsCorrect) {
525525
}
526526
}
527527
}
528+
529+
TEST(MachineInstr, HasSideEffects) {
530+
using namespace ARM;
531+
unsigned Opcodes[] = {
532+
// MVE Loads/Stores
533+
MVE_VLDRBS16, MVE_VLDRBS16_post, MVE_VLDRBS16_pre,
534+
MVE_VLDRBS16_rq, MVE_VLDRBS32, MVE_VLDRBS32_post,
535+
MVE_VLDRBS32_pre, MVE_VLDRBS32_rq, MVE_VLDRBU16,
536+
MVE_VLDRBU16_post, MVE_VLDRBU16_pre, MVE_VLDRBU16_rq,
537+
MVE_VLDRBU32, MVE_VLDRBU32_post, MVE_VLDRBU32_pre,
538+
MVE_VLDRBU32_rq, MVE_VLDRBU8, MVE_VLDRBU8_post,
539+
MVE_VLDRBU8_pre, MVE_VLDRBU8_rq, MVE_VLDRDU64_qi,
540+
MVE_VLDRDU64_qi_pre, MVE_VLDRDU64_rq, MVE_VLDRDU64_rq_u,
541+
MVE_VLDRHS32, MVE_VLDRHS32_post, MVE_VLDRHS32_pre,
542+
MVE_VLDRHS32_rq, MVE_VLDRHS32_rq_u, MVE_VLDRHU16,
543+
MVE_VLDRHU16_post, MVE_VLDRHU16_pre, MVE_VLDRHU16_rq,
544+
MVE_VLDRHU16_rq_u, MVE_VLDRHU32, MVE_VLDRHU32_post,
545+
MVE_VLDRHU32_pre, MVE_VLDRHU32_rq, MVE_VLDRHU32_rq_u,
546+
MVE_VLDRWU32, MVE_VLDRWU32_post, MVE_VLDRWU32_pre,
547+
MVE_VLDRWU32_qi, MVE_VLDRWU32_qi_pre, MVE_VLDRWU32_rq,
548+
MVE_VLDRWU32_rq_u, MVE_VLD20_16, MVE_VLD20_16_wb,
549+
MVE_VLD20_32, MVE_VLD20_32_wb, MVE_VLD20_8,
550+
MVE_VLD20_8_wb, MVE_VLD21_16, MVE_VLD21_16_wb,
551+
MVE_VLD21_32, MVE_VLD21_32_wb, MVE_VLD21_8,
552+
MVE_VLD21_8_wb, MVE_VLD40_16, MVE_VLD40_16_wb,
553+
MVE_VLD40_32, MVE_VLD40_32_wb, MVE_VLD40_8,
554+
MVE_VLD40_8_wb, MVE_VLD41_16, MVE_VLD41_16_wb,
555+
MVE_VLD41_32, MVE_VLD41_32_wb, MVE_VLD41_8,
556+
MVE_VLD41_8_wb, MVE_VLD42_16, MVE_VLD42_16_wb,
557+
MVE_VLD42_32, MVE_VLD42_32_wb, MVE_VLD42_8,
558+
MVE_VLD42_8_wb, MVE_VLD43_16, MVE_VLD43_16_wb,
559+
MVE_VLD43_32, MVE_VLD43_32_wb, MVE_VLD43_8,
560+
MVE_VLD43_8_wb, MVE_VSTRB16, MVE_VSTRB16_post,
561+
MVE_VSTRB16_pre, MVE_VSTRB16_rq, MVE_VSTRB32,
562+
MVE_VSTRB32_post, MVE_VSTRB32_pre, MVE_VSTRB32_rq,
563+
MVE_VSTRB8_rq, MVE_VSTRBU8, MVE_VSTRBU8_post,
564+
MVE_VSTRBU8_pre, MVE_VSTRD64_qi, MVE_VSTRD64_qi_pre,
565+
MVE_VSTRD64_rq, MVE_VSTRD64_rq_u, MVE_VSTRH16_rq,
566+
MVE_VSTRH16_rq_u, MVE_VSTRH32, MVE_VSTRH32_post,
567+
MVE_VSTRH32_pre, MVE_VSTRH32_rq, MVE_VSTRH32_rq_u,
568+
MVE_VSTRHU16, MVE_VSTRHU16_post, MVE_VSTRHU16_pre,
569+
MVE_VSTRW32_qi, MVE_VSTRW32_qi_pre, MVE_VSTRW32_rq,
570+
MVE_VSTRW32_rq_u, MVE_VSTRWU32, MVE_VSTRWU32_post,
571+
MVE_VSTRWU32_pre, MVE_VST20_16, MVE_VST20_16_wb,
572+
MVE_VST20_32, MVE_VST20_32_wb, MVE_VST20_8,
573+
MVE_VST20_8_wb, MVE_VST21_16, MVE_VST21_16_wb,
574+
MVE_VST21_32, MVE_VST21_32_wb, MVE_VST21_8,
575+
MVE_VST21_8_wb, MVE_VST40_16, MVE_VST40_16_wb,
576+
MVE_VST40_32, MVE_VST40_32_wb, MVE_VST40_8,
577+
MVE_VST40_8_wb, MVE_VST41_16, MVE_VST41_16_wb,
578+
MVE_VST41_32, MVE_VST41_32_wb, MVE_VST41_8,
579+
MVE_VST41_8_wb, MVE_VST42_16, MVE_VST42_16_wb,
580+
MVE_VST42_32, MVE_VST42_32_wb, MVE_VST42_8,
581+
MVE_VST42_8_wb, MVE_VST43_16, MVE_VST43_16_wb,
582+
MVE_VST43_32, MVE_VST43_32_wb, MVE_VST43_8,
583+
MVE_VST43_8_wb,
584+
};
585+
586+
LLVMInitializeARMTargetInfo();
587+
LLVMInitializeARMTarget();
588+
LLVMInitializeARMTargetMC();
589+
590+
auto TT(Triple::normalize("thumbv8.1m.main-arm-none-eabi"));
591+
std::string Error;
592+
const Target *T = TargetRegistry::lookupTarget(TT, Error);
593+
if (!T) {
594+
dbgs() << Error;
595+
return;
596+
}
597+
598+
TargetOptions Options;
599+
auto TM = std::unique_ptr<LLVMTargetMachine>(
600+
static_cast<LLVMTargetMachine *>(T->createTargetMachine(
601+
TT, "generic", "", Options, None, None, CodeGenOpt::Default)));
602+
ARMSubtarget ST(TM->getTargetTriple(), TM->getTargetCPU(),
603+
TM->getTargetFeatureString(),
604+
*static_cast<const ARMBaseTargetMachine *>(TM.get()), false);
605+
const ARMBaseInstrInfo *TII = ST.getInstrInfo();
606+
auto MII = TM->getMCInstrInfo();
607+
608+
for (unsigned Op : Opcodes) {
609+
const MCInstrDesc &Desc = TII->get(Op);
610+
ASSERT_FALSE(Desc.hasUnmodeledSideEffects())
611+
<< MII->getName(Op) << " has unexpected side effects";
612+
}
613+
}

0 commit comments

Comments
 (0)