[AArch64] Add preferred alignments for Exynos M1

[evandro]

Summary

Add the preferred alignments in the way done in other targets, namely in <Target>ISelLowering.cpp.

However, using AArch64Subtarget::getProcFamily() is discouraged.

Possible alternatives:

1. Make TargetLoweringBase::setPrefFunctionAlignment(), TargetLoweringBase::setPrefLoopAlignment(), etc, public.
2. Override the visibility of methods above in AArch64TargetLowering.
3. Add equivalent public methods in AArch64TargetLowering wrapping around them.
4. Create a parameterizeable SubtargetFeature (?)
5. Etc

[t.p.northover]

The code looks reasonably straightforward, but it ought to be fairly easily testable.

Tim.

[MatzeB]

This is how I would expect this to look to avoid getProcFamily():

https://ghostbin.com/paste/7cyoa

[MatzeB]

In D21203#453982, @MatzeB wrote:

This is how I would expect this to look to avoid getProcFamily():

https://ghostbin.com/paste/7cyoa

(And just to reduce the amount of code further) We should consider making all those properties in SubTargetInfo public and avoiding the getXXX() accessor functions there.

[MatzeB]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

645–652

The trouble with this is that you will not get a warning once you add a new CPU. (Let's say the successor to ExynosM1 benefits from the same setting). You could slightly improve this by not using default: break; but instead listing all CPUs so that you would at least get a warning about missing entries. Though the cleanest solution IMO is bundling all those per-CPU tweaks in subtarget info as shown in my ghostbin patch.

[evandro]

In D21203#453982, @MatzeB wrote:

This is how I would expect this to look to avoid getProcFamily():

https://ghostbin.com/paste/7cyoa

I see. I'll rip it off and update this proposal.

[evandro]

Gotcha!

[evandro]

In order to allow a subtarget to override some alignments, it was also necessary to add support for them in AArch64TargetLowering and AArch64Subtarget, after Matthias Braun's suggestion.

[MatzeB]

LGTM, thanks for switching to the "properties in SubTarget" style.

[MatzeB]

llvm/lib/Target/AArch64/AArch64Subtarget.h

89–90

You can probably use uint8_t instead of unsigned here.

[evandro]

llvm/lib/Target/AArch64/AArch64Subtarget.h

89–90

Sorry, I ended up forgetting about this one.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

AArch64TargetLowering::AArch64TargetLowering(const TargetMachine &TM,
   setStackPointerRegisterToSaveRestore(AArch64::SP);
 
   setSchedulingPreference(Sched::Hybrid);
 
   // Enable TBZ/TBNZ
   MaskAndBranchFoldingIsLegal = true;
   EnableExtLdPromotion = true;
 
+  // Set required alignment.
   setMinFunctionAlignment(2);
+  // Set preferred alignments.
+  setPrefFunctionAlignment(STI.getPrefFunctionAlignment());
+  setPrefLoopAlignment(STI.getPrefLoopAlignment());
 
   setHasExtractBitsInsn(true);
 
   setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::Other, Custom);
 
   if (Subtarget->hasNEON()) {
     // FIXME: v1f64 shouldn't be legal if we can avoid it, because it leads to
     // silliness like this:
for (MVT Ty : {MVT::v2f32, MVT::v4f32, MVT::v2f64}) {
       setOperationAction(ISD::FROUND, Ty, Legal);
     }
   }
 
   PredictableSelectIsExpensive = Subtarget->predictableSelectIsExpensive();
 }
 
 void AArch64TargetLowering::addTypeForNEON(MVT VT, MVT PromotedBitwiseVT) {
   if (VT == MVT::v2f32 || VT == MVT::v4f16) {
     setOperationAction(ISD::LOAD, VT, Promote);
     AddPromotedToType(ISD::LOAD, VT, MVT::v2i32);
 
     setOperationAction(ISD::STORE, VT, Promote);
     AddPromotedToType(ISD::STORE, VT, MVT::v2i32);
   } else if (VT == MVT::v2f64 || VT == MVT::v4f32 || VT == MVT::v8f16) {
     setOperationAction(ISD::LOAD, VT, Promote);
     AddPromotedToType(ISD::LOAD, VT, MVT::v2i64);
 
     setOperationAction(ISD::STORE, VT, Promote);
     AddPromotedToType(ISD::STORE, VT, MVT::v2i64);
   }
 
   // Mark vector float intrinsics as expand.
   if (VT == MVT::v2f32 || VT == MVT::v4f32 || VT == MVT::v2f64) {

llvm/lib/Target/AArch64/AArch64Subtarget.h

protected:
   bool DisableLatencySchedHeuristic = false;
   bool UseRSqrt = false;
   uint8_t MaxInterleaveFactor = 2;
   uint8_t VectorInsertExtractBaseCost = 3;
   uint16_t CacheLineSize = 0;
   uint16_t PrefetchDistance = 0;
   uint16_t MinPrefetchStride = 1;
   unsigned MaxPrefetchIterationsAhead = UINT_MAX;
+  unsigned PrefFunctionAlignment = 0;
+  unsigned PrefLoopAlignment = 0;
 
   // ReserveX18 - X18 is not available as a general purpose register.
   bool ReserveX18;
 
   bool IsLittle;
 
   /// CPUString - String name of used CPU.
   std::string CPUString;
unsigned getVectorInsertExtractBaseCost() const {
     return VectorInsertExtractBaseCost;
   }
   unsigned getCacheLineSize() const { return CacheLineSize; }
   unsigned getPrefetchDistance() const { return PrefetchDistance; }
   unsigned getMinPrefetchStride() const { return MinPrefetchStride; }
   unsigned getMaxPrefetchIterationsAhead() const {
     return MaxPrefetchIterationsAhead;
   }
+  unsigned getPrefFunctionAlignment() const { return PrefFunctionAlignment; }
+  unsigned getPrefLoopAlignment() const { return PrefLoopAlignment; }
 
   /// CPU has TBI (top byte of addresses is ignored during HW address
   /// translation) and OS enables it.
   bool supportsAddressTopByteIgnored() const;
 
   bool hasPerfMon() const { return HasPerfMon; }
   bool hasFullFP16() const { return HasFullFP16; }
   bool hasSPE() const { return HasSPE; }

llvm/lib/Target/AArch64/AArch64Subtarget.cpp

case Cyclone:
     CacheLineSize = 64;
     PrefetchDistance = 280;
     MinPrefetchStride = 2048;
     MaxPrefetchIterationsAhead = 3;
     break;
   case CortexA57:
     MaxInterleaveFactor = 4;
     break;
+  case ExynosM1:
+    PrefFunctionAlignment = 4;
+    PrefLoopAlignment = 3;
+    break;
   case Kryo:
     MaxInterleaveFactor = 4;
     VectorInsertExtractBaseCost = 2;
     break;
-  case Others: break;
   case CortexA35: break;
   case CortexA53: break;
-  case ExynosM1: break;
+  case Others: break;
   }
 }
 
 AArch64Subtarget::AArch64Subtarget(const Triple &TT, const std::string &CPU,
                                    const std::string &FS,
                                    const TargetMachine &TM, bool LittleEndian)
     : AArch64GenSubtargetInfo(TT, CPU, FS), ReserveX18(TT.isOSDarwin()),
       IsLittle(LittleEndian), CPUString(CPU), TargetTriple(TT), FrameLowering(),