[PATCH] D38597: [PEI] Remove required properties and use 'if' instead of std::function

[Derek Schuff via Phabricator via llvm-commits]

dschuff added a subscriber: qcolombet.
dschuff added a comment.

Removing the requirement works fine now because the passes are set up correctly. The original intention behind `getRequiredProperties` in this case was that if the pass configuration were to be changed and PEI were to be called before regalloc on a target which wasn't prepared for that, there would be an easy assertion to say what went wrong.

We could just add a conditional assertion in `runOnMachineFunction` to ensure things are correct; in fact my original patch to PEI for WebAssembly more or less did that. But my reviewer (@qcolombet ) really wanted a stronger mechanism to enforce that kind of invariant and IIRC that's why `getRequiredProperties` was born.

I guess if we don't have the target anymore at the right time, we can't really properly set `getRequiredProperties`. I'd be fine with going back to a simple assertion here, but I guess that risks re-opening the debate :)



================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:82
 private:
-  std::function<void(MachineFunction &MF, RegScavenger *RS,
-                     unsigned &MinCSFrameIndex, unsigned &MaxCSFrameIndex,
----------------
regardless of what we decide about `getRequiredProperties` we should do the rest of this change (i.e. this and below). There was some discussion about it after it originally landed and it was my intention to do more or less exactly this part of your patch, and I didn't get around to it :(


https://reviews.llvm.org/D38597