[RISCV] Remove RV32 HwMode. Use DefaultMode for RV32

[craig.topper]

Summary

Prior to this the DefaultMode was never selected, but RISCVGenDAGISel.inc, RISCVGenRegisterInfo.inc, RISCVGenGlobalISel.inc all ended up with extra table entries for that mode.

This patch removes the RV32 and uses DefaultMode for RV32. This impressively reduces the size of my release+asserts llc binary by about 270K. About 15K from RISCVGenDAGISel.inc, 1-2K from RISCVGenRegisterInfo.inc, but the vast majority from RISCVGenGlobalISel.inc.

[asb]

Ouch - I'm sorry to have contributed to the bloating of LLVM binaries!

This makes sense to me, but perhaps @kparzysz you could weigh in too? I'd copied the style used in Hexagon here, and it seems like the equivalent change could also be made to Hexagon.td and HexagonRegisterInfo.td?

[kparzysz]

Sorry, I just noticed this now. Yes, this does make perfect sense. As a matter of fact, I've been thinking of reorganizing the "default mode" to be something that does not need an explicit entry.

[craig.topper]

Any objections to me committing this?

[luismarques]

In D90973#2407169, @craig.topper wrote:

Any objections to me committing this?

No, please go ahead. Thank you.

[lenary]

llvm/lib/Target/RISCV/RISCV.td

184–185

Is there a way of doing an alias here, so we don't have to document that defaultmode is rv32 everywhere?

Something like the following (I guess it has to be a let, not a def, but I could be wrong).

let RV32 = DefaultMode;`

[lenary]

llvm/lib/Target/RISCV/RISCV.td

184–185

Sorry, it can't be a let, but you could do the following, I think

defvar RV32 = DefaultMode;

And then in an example pulled from below:

def XLenVT : ValueTypeByHwMode<[RV64, RV32],
                               [i64,  i32]>;

[craig.topper]

llvm/lib/Target/RISCV/RISCV.td

184–185

That works, so I'll switch to that.

[jrtc27]

llvm/lib/Target/RISCV/RISCV.td

184–185

Hm, does this work the other way round too (and if not, what happens)? Having RV64 before RV32 is not the usual order.

[craig.topper]

llvm/lib/Target/RISCV/RISCV.td

184–185

It does! Switched to that order in 9211da4215b6d48c8b9186b78274946789c559e9

[jrtc27]

llvm/lib/Target/RISCV/RISCV.td

184–185

Thanks! I always expect the worst when HwMode is involved...

[kparzysz]

llvm/lib/Target/RISCV/RISCV.td

184–185

I always expect the worst when HwMode is involved...

What's causing this? Is there anything specific that could be improved?

[jrtc27]

llvm/lib/Target/RISCV/RISCV.td

184–185

The main issues I see with HwMode are that, unless I am mistaken, all HwMode instances must be mutually-exclusive such that if you have two orthogonal modes you need to take the cross product of them manually, which is particularly problematic for downstream as that can cause significant diffs. Ideally you would be able to have separate "groups" of HwMode that are internally mutually-exclusive and TableGen could do the cross-product for you (or maybe something smarter so as to not bloat the generated code), or failing that you'd have to define the exploded list but then be able to define aliases as sets of HwMode's.

As a more minor thing there's also the fact that only a very limited set of fields can be HwMode-dependent, and at least downstream it would be convenient to have mode-dependent sub-register definitions; currently we just use SubRegIndex<-1, -1>.

[kparzysz]

llvm/lib/Target/RISCV/RISCV.td

184–185

The mutual exclusivity requirement is true, and there are a few reasons for that. The initial reason was ensuring deterministic behavior during instruction selection: selection patterns are applied in some order, and if one pattern fails, the next one is applied. Since HwMode expands into predicates, a single source code pattern becomes multiple ones in the matching code. There is no natural ordering between such clones that would be clear at the source code level, so with the "first matched, first serve" approach, you'd get unexpected behavior.

The HwMode was designed to be a single parameter that describes (enumerates) the configuration of the hardware. Historically, the initial motivation was handling 64- and 128-byte vectors on Hexagon with the same instruction set, then the 32- and 64-bit modes of RISCV became another potential user. Keeping other targets unaffected (i.e. not forcing them to change anything in their .td files) was a very significant factor in the design, or else this feature would have little chance of making it in.

Having aliases for sets of modes may actually not be hard to add. Could you show me an example of how you'd use it?

The way that TableGen handles registers and subregisters doesn't make things easy to customize. If I wanted to add parameterized subregister definitions, it would have likely affected the lane mask calculations, and have a chain of (unnecessarily desirable) side-effects that could be difficult to track down. The lack of handling subregisters is actually the part of the HwMode feature that I dislike, but I'm afraid it would take a while to implement it properly.

llvm/lib/Target/RISCV/RISCV.td

def Feature64Bit
     : SubtargetFeature<"64bit", "HasRV64", "true", "Implements RV64">;
 def IsRV64 : Predicate<"Subtarget->is64Bit()">,
                        AssemblerPredicate<(all_of Feature64Bit),
                        "RV64I Base Instruction Set">;
 def IsRV32 : Predicate<"!Subtarget->is64Bit()">,
                        AssemblerPredicate<(all_of (not Feature64Bit)),
                        "RV32I Base Instruction Set">;
 
+defvar RV32 = DefaultMode;
 def RV64           : HwMode<"+64bit">;
-def RV32           : HwMode<"-64bit">;
 
 def FeatureRV32E
     : SubtargetFeature<"e", "IsRV32E", "true",
                        "Implements RV32E (provides 16 rather than 32 GPRs)">;
 def IsRV32E : Predicate<"Subtarget->isRV32E()">,
                         AssemblerPredicate<(all_of FeatureRV32E)>;
 
 def FeatureRelax

llvm/lib/Target/RISCV/RISCVRegisterInfo.td

let RegAltNameIndices = [ABIRegAltName] in {
   def X27 : RISCVReg<27,"x27", ["s11"]>, DwarfRegNum<[27]>;
   def X28 : RISCVReg<28,"x28", ["t3"]>, DwarfRegNum<[28]>;
   def X29 : RISCVReg<29,"x29", ["t4"]>, DwarfRegNum<[29]>;
   def X30 : RISCVReg<30,"x30", ["t5"]>, DwarfRegNum<[30]>;
   def X31 : RISCVReg<31,"x31", ["t6"]>, DwarfRegNum<[31]>;
   }
 }
 
-def XLenVT : ValueTypeByHwMode<[RV32, RV64, DefaultMode],
-                               [i32,  i64,  i32]>;
+def XLenVT : ValueTypeByHwMode<[RV64, RV32],
+                               [i64,  i32]>;
 
 // The order of registers represents the preferred allocation sequence.
 // Registers are listed in the order caller-save, callee-save, specials.
 def GPR : RegisterClass<"RISCV", [XLenVT], 32, (add
     (sequence "X%u", 10, 17),
     (sequence "X%u", 5, 7),
     (sequence "X%u", 28, 31),
     (sequence "X%u", 8, 9),
     (sequence "X%u", 18, 27),
     (sequence "X%u", 0, 4)
   )> {
   let RegInfos = RegInfoByHwMode<
-      [RV32,              RV64,              DefaultMode],
-      [RegInfo<32,32,32>, RegInfo<64,64,64>, RegInfo<32,32,32>]>;
+      [RV64,              RV32],
+      [RegInfo<64,64,64>, RegInfo<32,32,32>]>;
 }
 
 def GPRX0 : RegisterClass<"RISCV", [XLenVT], 32, (add X0)> {
   let RegInfos = RegInfoByHwMode<
-      [RV32,              RV64,              DefaultMode],
-      [RegInfo<32,32,32>, RegInfo<64,64,64>, RegInfo<32,32,32>]>;
+      [RV64,              RV32],
+      [RegInfo<64,64,64>, RegInfo<32,32,32>]>;
 }
 
 // The order of registers represents the preferred allocation sequence.
 // Registers are listed in the order caller-save, callee-save, specials.
 def GPRNoX0 : RegisterClass<"RISCV", [XLenVT], 32, (add
     (sequence "X%u", 10, 17),
     (sequence "X%u", 5, 7),
     (sequence "X%u", 28, 31),
     (sequence "X%u", 8, 9),
     (sequence "X%u", 18, 27),
     (sequence "X%u", 1, 4)
   )> {
   let RegInfos = RegInfoByHwMode<
-      [RV32,              RV64,              DefaultMode],
-      [RegInfo<32,32,32>, RegInfo<64,64,64>, RegInfo<32,32,32>]>;
+      [RV64,              RV32],
+      [RegInfo<64,64,64>, RegInfo<32,32,32>]>;
 }
 
 def GPRNoX0X2 : RegisterClass<"RISCV", [XLenVT], 32, (add
     (sequence "X%u", 10, 17),
     (sequence "X%u", 5, 7),
     (sequence "X%u", 28, 31),
     (sequence "X%u", 8, 9),
     (sequence "X%u", 18, 27),
     X1, X3, X4
   )> {
   let RegInfos = RegInfoByHwMode<
-      [RV32,              RV64,              DefaultMode],
-      [RegInfo<32,32,32>, RegInfo<64,64,64>, RegInfo<32,32,32>]>;
+      [RV64,              RV32],
+      [RegInfo<64,64,64>, RegInfo<32,32,32>]>;
 }
 
 def GPRC : RegisterClass<"RISCV", [XLenVT], 32, (add
     (sequence "X%u", 10, 15),
     (sequence "X%u", 8, 9)
   )> {
   let RegInfos = RegInfoByHwMode<
-      [RV32,              RV64,              DefaultMode],
-      [RegInfo<32,32,32>, RegInfo<64,64,64>, RegInfo<32,32,32>]>;
+      [RV64,              RV32],
+      [RegInfo<64,64,64>, RegInfo<32,32,32>]>;
 }
 
 // For indirect tail calls, we can't use callee-saved registers, as they are
 // restored to the saved value before the tail call, which would clobber a call
 // address.
 def GPRTC : RegisterClass<"RISCV", [XLenVT], 32, (add
     (sequence "X%u", 5, 7),
     (sequence "X%u", 10, 17),
     (sequence "X%u", 28, 31)
   )> {
   let RegInfos = RegInfoByHwMode<
-      [RV32,              RV64,              DefaultMode],
-      [RegInfo<32,32,32>, RegInfo<64,64,64>, RegInfo<32,32,32>]>;
+      [RV64,              RV32],
+      [RegInfo<64,64,64>, RegInfo<32,32,32>]>;
 }
 
 def SP : RegisterClass<"RISCV", [XLenVT], 32, (add X2)> {
   let RegInfos = RegInfoByHwMode<
-      [RV32,              RV64,              DefaultMode],
-      [RegInfo<32,32,32>, RegInfo<64,64,64>, RegInfo<32,32,32>]>;
+      [RV64,              RV32],
+      [RegInfo<64,64,64>, RegInfo<32,32,32>]>;
 }
 
 // Floating point registers
 let RegAltNameIndices = [ABIRegAltName] in {
   def F0_F  : RISCVReg32<0, "f0", ["ft0"]>, DwarfRegNum<[32]>;
   def F1_F  : RISCVReg32<1, "f1", ["ft1"]>, DwarfRegNum<[33]>;
   def F2_F  : RISCVReg32<2, "f2", ["ft2"]>, DwarfRegNum<[34]>;
   def F3_F  : RISCVReg32<3, "f3", ["ft3"]>, DwarfRegNum<[35]>;