[PATCH] D35319: LSE Atomics reorg - Part I

Tim Northover via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 13 07:34:31 PDT 2017


t.p.northover added a comment.

> What does whatever mean, in this context?

The code looks vaguely plausible for defining how these are scheduled but I have no idea whether it's actually a performance improvement because I don't have access to any ThunderX internal documentation that describes its microarchitecture.



================
Comment at: lib/Target/AArch64/AArch64DeadRegisterDefinitionsPass.cpp:141-142
+
+    for (unsigned I = 0; I < ND; ++I) {
+      const MachineOperand &MO = MI.getOperand(I);
+      if (!MO.isReg() || !MO.isDef())
----------------
steleman wrote:
> t.p.northover wrote:
> > This loop seems too generic: we know what instructions we're looking at, and we know that the entire purpose is to replace a register with XZR/WZR -- the surrounding code isn't going to do that if it's not in that class.
> I do not understand what you are saying here.
> 
> The whole purpose of this function is to **prevent** the use of WZR/XZR, and **not** replace some other register with XZR/WZR, which would be wrong, as per ARM Errata.
> 
> 
> 
Yes, but all of these instructions do have some definition that would otherwise be eligible. And if they didn't then the function wouldn't be necessary in the first place because the main loop checks.


Repository:
  rL LLVM

https://reviews.llvm.org/D35319





More information about the llvm-commits mailing list