<div dir="ltr">Hi Quentin,<div class="gmail_extra"><br>Updated with the latest comments.</div><div class="gmail_extra"><br></div><div class="gmail_extra">BTW, for instructions updating NCZV flags, actually I think it still make sense to mark them as rematerializable. Essentially the algorithm of rematerialization should take care of the final decision. For example, an adds instruction can still be rematerialized if only the flag is not really used by any instruction at all on control flow, or the flag may be overridden by another adds instruction. But I think it's OK to do it conservatively at this moment, so I agree to remove all instructions updating flags.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">Thanks,</div><div class="gmail_extra">-Jiangning</div><div class="gmail_extra"><br><div class="gmail_quote">2014-07-09 22:28 GMT+08:00 Quentin Colombet <span dir="ltr"><<a href="mailto:qcolombet@apple.com" target="_blank">qcolombet@apple.com</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">Hi Jiangning,<br>
<br>
A few comments on the second patch (the first one looks good).<br>
<br>
+// FIXME: this implementation should be micro-architecture dependent, so a<br>
+// micro-architecture target hook should be introduced here in future.<br>
+bool AArch64InstrInfo::isAsCheapAsAMove(const MachineInstr *MI) const {<br>
+  if (Subtarget.isCortexA57() || Subtarget.isCortexA53()) {<br>
+    switch (MI->getOpcode()) {<br>
+    default:<br>
+      return false;<br>
[…]<br>
+  }<br>
+<br>
+  return MI->isAsCheapAsAMove();<br>
+}<br>
+<br>
<br>
To match LLVM guidelines, use an early exit when the subtarget does not match:<br>
if (!Subtarget.isCortexA57() && !Subtarget.isCortexA53())<br>
  return MI->isAsCheapAsAMove();<br>
switch (MI->getOpcode()) {<br>
[…]<br>
<br>
+    case AArch64::ANDSWri:<br>
+    case AArch64::ANDSXri:<br>
<br>
I believe this is a remaining of the initial patch. The S variant are not rematerializable.<br>
<br>
Thanks,<br>
-Quentin<br>
<br>
</div><a href="http://reviews.llvm.org/D4361" target="_blank">http://reviews.llvm.org/D4361</a><br>
<br>
<br>
</blockquote></div><br></div></div>