[PATCH] D95588: [RISCV] Implement the MC layer support of P extension

[Jim Lin via Phabricator via llvm-commits]

Jim marked 2 inline comments as done.
Jim added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoP.td:909
+
+// kmar64 has a aliased instruction kmada32 belong to zpn sub-extension on RV64.
+let DecoderNamespace = "RISCV32Zpsfoperand_",
----------------
Jim wrote:
> jrtc27 wrote:
> > Having the same instruction in two different extensions under two different names is insane. Currently this implementation lets you use the "wrong" name for kmar64 with Zpn. But I would prefer the spec weren't crazy.
> I am still working on fixing it (from spec or ...) . But it is spec issue.
Remove alias inst first. I will discuss with P-extension maintainer on this issue.
Could this issue be fixed by a future revision?


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.td:228
+
+def GPR32Pair : RegisterClass<"RISCV", [untyped], 64, (add GPR32Pairs)> {
+  let Size = 64;
----------------
jrtc27 wrote:
> Jim wrote:
> > jrtc27 wrote:
> > > Why is this untyped?
> > GPR32Pair has untyped type. 
> > In code generation, It captures some operation with i64 type supported by Zpsoperand to untyped during legalization. 
> > In my mind, untyped is used to be represented special type don't need any legalization.
> > I refer to ARMRegisterInfo.td that defines GPRPair with untyped.
> So, presumably it can't be i64 because that would then make i64 a legal type? Maybe that's fine, maybe it's not, but alternatively it could be typed as v2i32 here (can include v4i16 and v8i8 too) and cast if/when needed. Craig maybe you have thoughts on this? But untyped always feels like laziness to me that results in TableGen being unable to help you when it comes to type checking, with rare exceptions.
All of operation node having untyped operands are customized SDNode lowered in RISCVISelLowering.cpp.  And matched to a specific pattern. 
untyped couldn't be as v2i32, v4i16 or v8i8 in the same time. The operand type has been checked in custom lowering.

The lowering for operation with specific operand types (v2i32, v4i16, v8i8 or i64 in this case) would be set cutsom and lowered to customized SDNode (This SDNode has untyped operands). 
Different operand types with the same operation are lowered to different customized SDNode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95588