GlobalISel/InstructionSelector: Implement GIR_CopyFConstantAsFPImm

[tstellar]

Summary

AMDGPU will need this to enable the TableGen'd GlobalISel selector.

[dsanders]

LGTM. Preferably with a test case in this patch but adding the test cases along with the AMDGPU patches you're working on will also be ok

[tstellar]

I actually don't have a test case of this. I really just need the enum defined so AMDGPUGenGlobalIsel.inc will build. I can drop the implementation and just add the enum value if you prefer.

[dsanders]

In D45990#1084299, @tstellar wrote:

I actually don't have a test case of this. I really just need the enum defined so AMDGPUGenGlobalIsel.inc will build. I can drop the implementation and just add the enum value if you prefer.

Could you send me the comment that's shortly before the GIR_CopyFConstantAsFPImm in AMDGPUGenGlobalIsel.inc? It should look similar to a tablegen pattern and we can probably derive a test case from that. I'm not sure which rule has been imported but I'm hoping it will be something like (set $rd:GPR, $imm:fpimm) in which case the test would be similar to test/CodeGen/AArch64/GlobalISel/select-constant.mir.

[tstellar]

In D45990#1084322, @dsanders wrote:

In D45990#1084299, @tstellar wrote:

I actually don't have a test case of this. I really just need the enum defined so AMDGPUGenGlobalIsel.inc will build. I can drop the implementation and just add the enum value if you prefer.

Could you send me the comment that's shortly before the GIR_CopyFConstantAsFPImm in AMDGPUGenGlobalIsel.inc? It should look similar to a tablegen pattern and we can probably derive a test case from that. I'm not sure which rule has been imported but I'm hoping it will be something like (set $rd:GPR, $imm:fpimm) in which case the test would be similar to test/CodeGen/AArch64/GlobalISel/select-constant.mir.

Here is the comment. Note that the reason I don't have a test case is because this pattern is only for the R600 sub-target and I'm not implementing global-isel for that sub-target:

// MIs[0] Operand 1
// No operand predicates
// (fpimm:{ *:[f32] }):$val  =>  (MOV_IMM_F32:{ *:[f32] } (fpimm:{ *:[f32] }):$val)
GIR_BuildMI, /*InsnID*/0, /*Opcode*/AMDGPU::MOV_IMM_F32,
GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/0, // dst
GIR_CopyFConstantAsFPImm, /*NewInsnID*/0, /*OldInsnID*/0, // va

[dsanders]

In D45990#1084328, @tstellar wrote:

In D45990#1084322, @dsanders wrote:

In D45990#1084299, @tstellar wrote:

I actually don't have a test case of this. I really just need the enum defined so AMDGPUGenGlobalIsel.inc will build. I can drop the implementation and just add the enum value if you prefer.

Could you send me the comment that's shortly before the GIR_CopyFConstantAsFPImm in AMDGPUGenGlobalIsel.inc? It should look similar to a tablegen pattern and we can probably derive a test case from that. I'm not sure which rule has been imported but I'm hoping it will be something like (set $rd:GPR, $imm:fpimm) in which case the test would be similar to test/CodeGen/AArch64/GlobalISel/select-constant.mir.

Here is the comment. Note that the reason I don't have a test case is because this pattern is only for the R600 sub-target and I'm not implementing global-isel for that sub-target:

// MIs[0] Operand 1
// No operand predicates
// (fpimm:{ *:[f32] }):$val  =>  (MOV_IMM_F32:{ *:[f32] } (fpimm:{ *:[f32] }):$val)
GIR_BuildMI, /*InsnID*/0, /*Opcode*/AMDGPU::MOV_IMM_F32,
GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/0, // dst
GIR_CopyFConstantAsFPImm, /*NewInsnID*/0, /*OldInsnID*/0, // va

Thanks, something similar select-constant.mir would do the trick for that one but seeing as it's for a target that you aren't implementing it makes sense to go ahead without it. I think we might as well keep the implementation seeing as we have it but could you add a TODO indicating that it needs a test case as soon as one is possible?

llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelector.h

enum {
   GIR_CustomRenderer,
 
   /// Render a G_CONSTANT operator as a sign-extended immediate.
   /// - NewInsnID - Instruction ID to modify
   /// - OldInsnID - Instruction ID to copy from
   /// The operand index is implicitly 1.
   GIR_CopyConstantAsSImm,
 
+  /// Render a G_FCONSTANT operator as a sign-extended immediate.
+  /// - NewInsnID - Instruction ID to modify
+  /// - OldInsnID - Instruction ID to copy from
+  /// The operand index is implicitly 1.
+  GIR_CopyFConstantAsFPImm,
+
   /// Constrain an instruction operand to a register class.
   /// - InsnID - Instruction ID to modify
   /// - OpIdx - Operand index
   /// - RCEnum - Register class enumeration value
   GIR_ConstrainOperandRC,
   /// Constrain an instructions operands according to the instruction
   /// description.
   /// - InsnID - Instruction ID to modify

llvm/trunk/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h

case GIR_CopyConstantAsSImm: {
       else
         llvm_unreachable("Expected Imm or CImm operand");
       DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
                       dbgs() << CurrentIdx << ": GIR_CopyConstantAsSImm(OutMIs["
                              << NewInsnID << "], MIs[" << OldInsnID << "])\n");
       break;
     }
 
+    // TODO: Needs a test case once we have a pattern that uses this.
+    case GIR_CopyFConstantAsFPImm: {
+      int64_t NewInsnID = MatchTable[CurrentIdx++];
+      int64_t OldInsnID = MatchTable[CurrentIdx++];
+      assert(OutMIs[NewInsnID] && "Attempted to add to undefined instruction");
+      assert(State.MIs[OldInsnID]->getOpcode() == TargetOpcode::G_FCONSTANT && "Expected G_FCONSTANT");
+      if (State.MIs[OldInsnID]->getOperand(1).isFPImm())
+        OutMIs[NewInsnID].addFPImm(
+            State.MIs[OldInsnID]->getOperand(1).getFPImm());
+      else
+        llvm_unreachable("Expected FPImm operand");
+      DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
+                      dbgs() << CurrentIdx << ": GIR_CopyFPConstantAsFPImm(OutMIs["
+                             << NewInsnID << "], MIs[" << OldInsnID << "])\n");
+      break;
+    }
+
     case GIR_CustomRenderer: {
       int64_t InsnID = MatchTable[CurrentIdx++];
       int64_t OldInsnID = MatchTable[CurrentIdx++];
       int64_t RendererFnID = MatchTable[CurrentIdx++];
       assert(OutMIs[InsnID] && "Attempted to add to undefined instruction");
       DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
                       dbgs() << CurrentIdx << ": GIR_CustomRenderer(OutMIs["
                              << InsnID << "], MIs[" << OldInsnID << "], "