[PATCH] D17011: [lanai] Add Lanai backend.

[Jacques Pienaar via llvm-commits]

jpienaar added a comment.

Hey,

Thanks for the thorough review and good suggestions.

Jacques


================
Comment at: lib/Target/Lanai/LanaiInstrInfo.td:429-431
@@ +428,5 @@
+  let H = 0 in
+    def SHL_I : ShiftRI<"sh", 0, shl, immZExt6>;
+  let H = 1 in
+    def SHA_I : ShiftRI<"sha", 0, shl, immZExt6>;
+  let H = 0 in
----------------
t.p.northover wrote:
> I think the shifts are a bit messy (especially the immZExt6 and immNegZext6 Operands which don't seem to agree between any 2 places they're mentioned).
> 
> The comments in the formats file say that imm16 is sign extended, and only values between -31 and 31 are valid.
> 
> What I'd suggest is unifying SHL_I/SRL_I and SHA_I/SRA_I. Their joint operand (called shift_imm, say, because it's not really a 2s-complement imm6 in any sense) then just needs to check "Imm >= -31 && Imm <= 31". You might want to check validity in the decoder, but other than that weird helpers ought to be unnecessary.
> 
> This range works for CodeGen too, based on the shl node (negative shifts get weird, but they're UB anyway).
> 
> Finally, you'll need separate "def : Pat" patterns to handle srl/sra DAGs.
Ha, yes I was just talking about changing this, this past week. But I wanted to avoid too much changes while you were reviewing. Updated.


http://reviews.llvm.org/D17011