[PATCH] [AArch64] Enhance rematerialization by adding a new API isAsCheapAsAMove in TargetInstroInfo
qcolombet at apple.com
Thu Jul 10 06:52:07 PDT 2014
The patch looks good with one nitpick however, I think we need a bit more discussion before we commit both patches.
First the nitpick:
+ case AArch64::ORNWrr:
+ case AArch64::ORNXrr:
+ case AArch64::ORRWrr:
+ case AArch64::ORRXrr:
+ return true;
// <— use llvm_unreachable here, just to make sure we do not miss that when updating the code.
Going back to the concern that we are conflating isRematerializable and isAsCheapAsAMove, I can see why we want to keep both. That said, I still think that if we are overriding isAsCheapAsMove with this new hook for isRematerializable instructions, those instructions should be marked as isAsCheapAsMove too.
This means two things:
1. All the uses of MachineInstr::isAsCheapAsMove should be promoted to use of TargetInstrInfo::isAsCheapAsMove.
2. If an instruction is rematerializable but not as cheap as move, then it shouldn’t appear in the isAsCheapAsMove target hook.
If you have an instruction matching #2 and still want to make it appear in TargetInstrInfo::isAsCheapAsMove, it means you are trying to abuse the register coalescer to do rematerialization and that’s probably not the right solution.
What do you think?
More information about the llvm-commits