[PATCH] D124308: [MachineScheduler] exclude INLINEASM from schedule when it would increase register pressure

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
Comment at: llvm/lib/Target/X86/X86RegisterInfo.cpp:266-271
+  case X86::GR8RegClassID:
+  case X86::GR16RegClassID:
   case X86::GR32RegClassID:
     return 4 - FPDiff;
   case X86::GR64RegClassID:
     return 12 - FPDiff;
----------------
pengfei wrote:
> Should we change them to `(Is64Bit ? 12 : 4) - FPDiff` ?
Yeah, I think so. Though I'm curious about the use of 4 in the first place. i386 has:
* eax
* ebx
* ecx
* edx
* esi
* edi
* ebp
* esp
* eip

As GPRs. eip and esp aren't allocatable, and ebp is the condition `FPDiff` here.  Why 4? I would have guessed 6 for the first 6 (for `-m32`)?

Perhaps 4 corresponds to `GR32_ABCD`, for some reason? So should the cases used be `GR32_ABCDRegClassID` rather than `GR8RegClassID`? Or should it be `GR8RegClassID` then use `6 - FPDiff`?

For 64b, I'd have guessed those six plus r8d to r15d, for a total of 14, not 12.  Am I missing something (perhaps about esi+edi)?


================
Comment at: llvm/lib/Target/X86/X86RegisterInfo.cpp:275
   case X86::VR64RegClassID:
     return 4;
   }
----------------
pengfei wrote:
> I wonder why return 4 for it.
Good question. Some comments could be added to this function to help retain more context as to WHY these magic constants have these values. I'm happy to do so in these commits, if we can document _why_.

I'm running `git log -S X86TargetLowering::getRegPressureLimit` to see if I can find additional context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124308