[PATCH] D80545: [AMDGPU/MemOpsCluster] Let mem ops clustering logic also consider number of clustered bytes

Mahesha S via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 26 09:11:42 PDT 2020


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


================
Comment at: llvm/lib/Target/AMDGPU/SIInsertHardClauses.cpp:159-166
+        unsigned WidthA = CI.Last
+                              ? !CI.Last->memoperands_empty()
+                                    ? CI.Last->memoperands().front()->getSize()
+                                    : 0
+                              : 0;
+        unsigned WidthB =
+            !MI.memoperands_empty() ? MI.memoperands().front()->getSize() : 0;
----------------
arsenm wrote:
> It would be better to not depend on the memory operands here, but this belongs in a helper function some kind of not (and this can also sink down to the use)
Hi @arsenm 

Did you mean here the helper function as a kind of below?

```
unsigned getDstMemOperandSize(const MachineInstr *MI) const {
  if (!MI || MI->memoperands_empty())
    return 0;

  return MI->memoperands().front()->getSize();
}
```

And, use above helper function as below?


```
unsigned WidthA = getDstMemOperandSize(CI.Last);
unsigned WidthB = getDstMemOperandSize(&MI);
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80545





More information about the llvm-commits mailing list