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

Authored by asb on Sat, Nov 11, 9:37 AM.

Details

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.

Diff Detail

Repository
rL LLVM
asb created this revision.Sat, Nov 11, 9:37 AM
rnk accepted this revision.Mon, Nov 13, 10:26 AM

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.

This revision is now accepted and ready to land.Mon, Nov 13, 10:26 AM
asb updated this revision to Diff 122719.Mon, Nov 13, 1:30 PM
asb edited the summary of this revision. (Show Details)

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

This revision was automatically updated to reflect the committed changes.