Set hasSideEffects=0 for TargetOpcode::{CFI_INSTRUCTION,EH_LABEL,GC_LABEL,ANNOTATION_LABEL}

[asb]

Summary

D37065 (committed as rL317674) explicitly set hasSideEffects for all TargetOpcode::* instructions where it was inferred previously. This is a follow-up to that patch, setting hasSideEffects=0 for CFI_INSTRUCTION, EH_LABEL, GC_LABEL and ANNOTATION_LABEL. All LLVM tests pass after this change.

This patch also modifies MachineInstr::isLabel returns true for a TargetOpcode::ANNOTATION_LABEL, which ensures that an annotation label won't be incorrectly considered safe to move.

[rnk]

Looks good. I think MachineInstr::isPosition is a more accurate way to identify these types of label instructions. If there is any code that relies on hasUnmodeledSideEffects, we can update it to check isPosition instead.

include/llvm/Target/Target.td

933 ↗

(On Diff #122587)

Can you check this opcode in MachineInstr::isLabel? I think I forgot to do that when I added it. Otherwise, I worry that it MachineInstr::isSafeToMove will return true for ANNOTATION_LABEL after this change.

[asb]

Thanks Reid, updated the patch with that change. I'll aim to commit tomorrow.

llvm/trunk/include/llvm/CodeGen/MachineInstr.h

public:
   /// Unlink 'this' form its basic block and delete it.
   ///
   /// If the instruction is part of a bundle, the other instructions in the
   /// bundle remain bundled.
   void eraseFromBundle();
 
   bool isEHLabel() const { return getOpcode() == TargetOpcode::EH_LABEL; }
   bool isGCLabel() const { return getOpcode() == TargetOpcode::GC_LABEL; }
+  bool isAnnotationLabel() const {
+    return getOpcode() == TargetOpcode::ANNOTATION_LABEL;
+  }
 
   /// Returns true if the MachineInstr represents a label.
-  bool isLabel() const { return isEHLabel() || isGCLabel(); }
+  bool isLabel() const {
+    return isEHLabel() || isGCLabel() || isAnnotationLabel();
+  }
 
   bool isCFIInstruction() const {
     return getOpcode() == TargetOpcode::CFI_INSTRUCTION;
   }
 
   // True if the instruction represents a position in the function.
   bool isPosition() const { return isLabel() || isCFIInstruction(); }
 

llvm/trunk/include/llvm/Target/Target.td

def INLINEASM : Instruction {
   let AsmString = "";
   let hasSideEffects = 0;  // Note side effect is encoded in an operand.
 }
 def CFI_INSTRUCTION : Instruction {
   let OutOperandList = (outs);
   let InOperandList = (ins i32imm:$id);
   let AsmString = "";
   let hasCtrlDep = 1;
-  let hasSideEffects = 1;
+  let hasSideEffects = 0;
   let isNotDuplicable = 1;
 }
 def EH_LABEL : Instruction {
   let OutOperandList = (outs);
   let InOperandList = (ins i32imm:$id);
   let AsmString = "";
   let hasCtrlDep = 1;
-  let hasSideEffects = 1;
+  let hasSideEffects = 0;
   let isNotDuplicable = 1;
 }
 def GC_LABEL : Instruction {
   let OutOperandList = (outs);
   let InOperandList = (ins i32imm:$id);
   let AsmString = "";
   let hasCtrlDep = 1;
-  let hasSideEffects = 1;
+  let hasSideEffects = 0;
   let isNotDuplicable = 1;
 }
 def ANNOTATION_LABEL : Instruction {
   let OutOperandList = (outs);
   let InOperandList = (ins i32imm:$id);
   let AsmString = "";
   let hasCtrlDep = 1;
-  let hasSideEffects = 1;
+  let hasSideEffects = 0;
   let isNotDuplicable = 1;
 }
 def KILL : Instruction {
   let OutOperandList = (outs);
   let InOperandList = (ins variable_ops);
   let AsmString = "";
   let hasSideEffects = 0;
 }