[PATCH] D48600: [GISel]:Add Opcodes for CTLZ/CTTZ/CTPOP

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: include/llvm/Support/TargetOpcodes.def:470-471
+
+/// Same as above, undefined for zero inputs.
+HANDLE_TARGET_OPCODE(G_CTTZ_ZERO_UNDEF)
+
----------------
aditya_nandakumar wrote:
> arsenm wrote:
> > Should we really preserve this SelectionDAGism or just make the zero is undef bit be an immediate operand to the instruction like the intrinsic?
> I think having two opcodes is explicit and is a little easier to work with - we don't usually have immediates in GISel instructions - and constants are encoded through a reg defining G_CONSTANT which makes it a little inconvenient to use. It probably also might make importing tablegen patterns slightly easier.
> I'm okay with either way if someone has a strong enough opinion. 
We'll need it for legalization. Typically only one or the other is legal and we can't do context-dependent legalization.


================
Comment at: test/TableGen/trydecode-emission.td:37
 // CHECK:      /* 0 */       MCD::OPC_ExtractField, 4, 4,  // Inst{7-4} ...
-// CHECK-NEXT: /* 3 */       MCD::OPC_FilterValue, 0, 14, 0, // Skip to: 21
-// CHECK-NEXT: /* 7 */       MCD::OPC_CheckField, 2, 2, 0, 5, 0, // Skip to: 18
-// CHECK-NEXT: /* 13 */      MCD::OPC_TryDecode, {{[0-9]+}}, 0, 0, 0, // Opcode: InstB, skip to: 18
-// CHECK-NEXT: /* 18 */      MCD::OPC_Decode, {{[0-9]+}}, 1, // Opcode: InstA
-// CHECK-NEXT: /* 21 */      MCD::OPC_Fail,
+// CHECK-NEXT: /* 3 */       MCD::OPC_FilterValue, 0, 16, 0, // Skip to: 23
+// CHECK-NEXT: /* 7 */       MCD::OPC_CheckField, 2, 2, 0, 6, 0, // Skip to: 19
----------------
aditya_nandakumar wrote:
> aemerson wrote:
> > Why are these tests changing?
> I believe these changes are due to increase in the number of opcodes.
We hit this out-of-tree a couple weeks ago, most of the changes are because the TryDecode/Decode are one byte larger and those are one byte larger because there are more opcodes before InstA.


================
Comment at: test/TableGen/trydecode-emission.td:39-40
+// CHECK-NEXT: /* 7 */       MCD::OPC_CheckField, 2, 2, 0, 6, 0, // Skip to: 19
+// CHECK-NEXT: /* 13 */      MCD::OPC_TryDecode, {{[0-9]+}}, 1, 0, 0, 0, // Opcode: InstB, skip to: 19
+// CHECK-NEXT: /* 19 */      MCD::OPC_Decode, {{[0-9]+}}, 1, 1, // Opcode: InstA
+// CHECK-NEXT: /* 23 */      MCD::OPC_Fail,
----------------
The new bytes should be checked with {{[0-9]+}}


Repository:
  rL LLVM

https://reviews.llvm.org/D48600