[PATCH] D52019: [AMDGPU] Divergence driven instruction selection. Part 1.

[Alexander via Phabricator via llvm-commits]

alex-t added inline comments.


================
Comment at: lib/Target/AMDGPU/VOP2Instructions.td:364
+let isConvergent = 1, Uses = []<Register> in {
+def V_READLANE_B32 : VOP2_Pseudo<"v_readlane_b32", VOP_READLANE,
+  [(set i32:$vdst, (int_amdgcn_readlane i32:$src0, i32:$src1))], "">;
----------------
rampitec wrote:
> Is there a reason to move it from its original place? This seems to grow the patch unnecessary and makes review harder.
In fact the reason was that Tablegen gen-asm-matcher fails with the empty prefix strings

in a def like this the empty string "" overrides the default VOP2_Pseido "prefix" argument that is prefix = "_e32"
VOP2_Pseudo <"v_madmk_f32", VOP_MADMK_F32, [], "">

As a result asm-matcher-gen complained about mnemonic alias with the same string.

For some miracle reason this expressed only with Predicates[] explicitly set.
I don't know how it is related. Some another one Tablegen black magic.

I moved this 4 defs back but removed empty prefix "". Since nothing failed after this I conclude that the empty prefix string was a covered bug.


================
Comment at: lib/Target/AMDGPU/VOP2Instructions.td:515
+      (getDivergentFrag<Op>.ret i64:$src0, i64:$src1),
+      (REG_SEQUENCE VReg_64,
+        (Inst  
----------------
rampitec wrote:
> Why is this reg_sequence and extract_subregs are needed? Why not just use i64 operands?
How can I use i64 operands with V_AND_B32 that operates i32?


================
Comment at: lib/Target/AMDGPU/VOPInstructions.td:576
+class OpOrNull<SDPatternOperator Op, VOPProfile P> {
+  SDPatternOperator ret = !if(!eq(P.NeedPatGen, 1), null_frag, Op);
+}
----------------
rampitec wrote:
> Where do you set NeedPatGen = 1?
Окау. All the stuff related this flag is going to be used later on when I add the patterns for extended encoding forms. I agree that it should be removed from this patch.


https://reviews.llvm.org/D52019