[PATCH] D90973: [RISCV] Remove RV32 HwMode. Use DefaultMode for RV32

Jessica Clarke via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 20 14:10:27 PST 2020


jrtc27 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCV.td:182-183
 
+// RV32 uses DefaultMode.
 def RV64           : HwMode<"+64bit">;
 
----------------
kparzysz wrote:
> jrtc27 wrote:
> > craig.topper wrote:
> > > jrtc27 wrote:
> > > > craig.topper wrote:
> > > > > lenary wrote:
> > > > > > lenary wrote:
> > > > > > > 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;`
> > > > > > > ```
> > > > > > 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]>;
> > > > > > ```
> > > > > That works, so I'll switch to that.
> > > > Hm, does this work the other way round too (and if not, what happens)? Having RV64 before RV32 is not the usual order.
> > > It does! Switched to that order in 9211da4215b6d48c8b9186b78274946789c559e9
> > Thanks! I always expect the worst when HwMode is involved...
> > I always expect the worst when HwMode is involved...
> 
> What's causing this?  Is there anything specific that could be improved?
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>`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90973/new/

https://reviews.llvm.org/D90973



More information about the llvm-commits mailing list