[PATCH] D69737: [PGO][PGSO] TargetInstrInfo part.

[Hiroshi Yamauchi via Phabricator via llvm-commits]

yamauchi marked an inline comment as done.
yamauchi added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetInstrInfo.h:136-138
+  virtual MachineInstr *commuteInstructionImpl(MachineInstr &MI,
+                                               ProfileSummaryInfo *PSI,
+                                               const MachineBlockFrequencyInfo *MBFI,
----------------
arsenm wrote:
> yamauchi wrote:
> > arsenm wrote:
> > > Why is this needed for commuting?
> > There isn't anything particular about commuting, but wherever we make a size-vs-speed code gen choice (one in X86InstrInfo.cpp), and if we want to do it in a profile guided manner, we'd need to propagate profile (PSI/MBFI) down.
> I don’t think it’s appropriate to pass this in to commuteInstruction. It should just perform the commute and not concern itself with profitability. It would be the caller’s responsibility to determine if commuting is a good idea 
Around X86InstrInfo.cpp:1576,

  case X86::BLENDPDrri:
  case X86::BLENDPDrri:
  case X86::BLENDPSrri:
  case X86::VBLENDPDrri:
  case X86::VBLENDPSrri: {
    // If we're optimizing for size, try to use MOVSD/MOVSS.
    if (MI.getParent()->getParent()->getFunction().hasOptSize()) {
       ....
    }

this code tries to replace BLEND with MOVSD, if possible, under optsize.

This is unfortunately internal to Target/X86InstrInfo (TII) and isn't up to the caller, except X86InstrInfo internally checks for optsize on the function.

This patch would like to make it profile guided (eg. do this for cold code, even without optsize) and it needs access to the profile (PSI/MBFI) there **in some way**.

It doesn't seem like a good idea to pass the profile to TII per pass or function because it is TII is created once and stateless.

There doesn't seem a good way to access the profile directly from TII because the profile isn't attached to the function or the IR.

Do you see some other way to accomplish this?

We could consider instead passing "bool OptSize" to commuteInstruction. What do you think?



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69737/new/

https://reviews.llvm.org/D69737