[PATCH] D94406: [AMDGPU] Fix failing assert with scratch ST mode

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

In D94406#2490053 <https://reviews.llvm.org/D94406#2490053>, @sebastian-ne wrote:

>> I think the intention was that getMemOperandsWithOffsetWidth should return false if no base operands were found (perhaps MachineScheduler could assert this?)
>
> I’d argue that in this case, we found the base operands and we know that except for the constant offset, they are zero.
> Also, we want these operations to be claused, I think this is the likely case for stack accesses in entry-points.

OK, sounds reasonable.

> Should we adjust the documentation comment to something like this?
>
>> It returns false if no base operands and offset was found.
>
> → It returns false if no base operands and offset could be determined.

That's a very subtle change! I think it would be better to be more explicit, e.g.:

  /// Get zero or more base operands and the byte offset of an instruction that reads/writes
  /// memory. Note that there may be zero base operands if the instruction accesses a constant address.

(Second sentence optional depending on how verbose you're feeling.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94406