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



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

asb created this revision.Nov 11 2017, 9:37 AM
rnk accepted this revision.Nov 13 2017, 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.

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.Nov 13 2017, 10:26 AM
asb updated this revision to Diff 122719.Nov 13 2017, 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.