[PATCH] D72758: Add OffsetIsScalable to getMemOperandWithOffset

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.
Herald added subscribers: kerbowa, wuzish.


================
Comment at: llvm/include/llvm/CodeGen/TargetInstrInfo.h:1245
                                        int64_t &Offset,
+                                       bool &OffsetIsScalable,
                                        const TargetRegisterInfo *TRI) const {
----------------
I'm not sure why this modification is necessary.  The offset doesn't actually scale, does it?  The width of memory operation does, but getMemOperandWithOffset doesn't return that anyway.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:957
+    if (BaseOpA->isIdenticalTo(*BaseOpB) &&
+        OffsetAIsScalable == OffsetBIsScalable) {
       int LowOffset = OffsetA < OffsetB ? OffsetA : OffsetB;
----------------
Is computing overlap on scalable vectors here really sound?


================
Comment at: llvm/lib/Target/Lanai/LanaiInstrInfo.h:78
                                     int64_t &Offset, unsigned &Width,
+                                    bool &OffsetIsScalable,
                                     const TargetRegisterInfo *TRI) const;
----------------
Do we need to change getMemOperandWithOffsetWidth now?  It's a target-specific method.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72758