[PATCH] D13085: AMDGPU: Simplify VOP2 operand legalization

Matt Arsenault via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 6 13:03:00 PDT 2015


arsenm added inline comments.

================
Comment at: lib/Target/AMDGPU/SIInstrInfo.cpp:802-805
@@ +801,6 @@
+
+  if (isVOP2(MI->getOpcode())) {
+    const MCInstrDesc &InstrDesc = get(MI->getOpcode());
+    // For VOP2 instructions, any operand type is valid to use for src0.  Make
+    // sure we can use the src1 as src0.
+    //
----------------
tstellarAMD wrote:
> This isn't true for instructions with implicit defs/uses of VCC. 
> 
> I prefer the explicit operand checking for legalization rather than make assumptions based on the instruction type, because there are usually one or two corner cases for each instruction type.
This isn't a concern here because commuteInstruction shouldn't ever see an illegal operand combination / constant bus restriction violation. Part of this set of changes it separating commuting for the common code's purposes (e.g. MachineCSE) and operand legalization. The only times constant bus violations are OK is coming out of isel and as an intermediate step during moveToVALU. This function shouldn't be used for legalization purposes. The constant bus restrictions should only be handled in legalizeOperands, although I need to double check if the VCC check is handled there already. For the implicit VCC cases, we should probably use an operand type that only includes VGPRs + inline immediate for src0/src1.


http://reviews.llvm.org/D13085





More information about the llvm-commits mailing list