[PATCH] [TargetInstrInfo] Add new hook: getMemOpBaseRegImmOfs.

[Sanjoy Das]

================
Comment at: include/llvm/Target/TargetInstrInfo.h:835-838
@@ -834,1 +834,6 @@
 
+  /// Get the base register and byte offset of an instruction that reads/writes
+  /// memory.  This is similar to getLdStBaseRegImmOfs, but also works on memory
+  /// instructions that have been folded into other non-memory operations, like
+  /// arithmetic.
+  virtual bool getMemOpBaseRegImmOfs(MachineInstr *MemOp, bool &IsLoadingOp,
----------------
ab wrote:
> Seems to me this should just reuse getLdStBaseRegImmOfs.  Judging by the user of that callback, the MachineScheduler, whether a load/store was folded or standalone isn't interesting.
> 
> Though if you override that for X86, it means the scheduler behavior might change, which is a whole 'nother can of worms.
> 
> Andy knows best;  +Matthias, who I believe has been looking into the scheduler.
> Though if you override that for X86, it means the scheduler behavior might change, which is a whole 'nother can of worms.

The only user of `getLdStBaseRegImmOfs` is `LoadClusterMotion`.  `LoadClusterMotion` kicks in only if `TargetInstrInfo::enableClusterLoads` returns `true`, and it returns `false` for x86.  So I think overriding `getLdStBaseRegImmOfs` for x86 should also be a NFC.

I did not override `getLdStBaseRegImmOfs` because the name seems to imply that we're parsing a load or a store; but I suppose just adding a clear comment should be fine here.

================
Comment at: lib/Target/X86/X86InstrInfo.cpp:3967-3986
@@ +3966,22 @@
+                                         const TargetRegisterInfo *TRI) const {
+  unsigned MemRefBegin = -1;
+
+  switch (MemOp->getOpcode()) {
+  default:
+    return false; // cannot parse this instruction
+
+  case X86::MOV64rm:
+  case X86::MOV32rm:
+  case X86::MOV16rm:
+    IsLoadingOp = true;
+    MemRefBegin = 1;
+    break;
+
+  case X86::ADD64rm:
+  case X86::ADD32rm:
+  case X86::ADD16rm:
+    IsLoadingOp = true;
+    MemRefBegin = 2;
+    break;
+  }
+
----------------
ab wrote:
> How about using X86II::getMemoryOperandNo() ?
> 
> isLoadingOp can be inferred from MC flags (MayLoad, or !MayStore, depending on what you're interested in).
> How about using X86II::getMemoryOperandNo() ?

Perfect, thanks for pointing that out!

http://reviews.llvm.org/D10199

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/