[PATCH] D11370: Improved the interface of methods commuting operands, improved X86-FMA3 mem-folding&coalescing.

[Quentin Colombet via llvm-commits]

qcolombet accepted this revision.
qcolombet added a comment.
This revision is now accepted and ready to land.

Hi Slava,

LGTM.

> The code 'if (MI->IsCommutable() && TII->commuteInstruction(MI))' works differently before and after the changes in commuteInstruction() method .


Thanks for the explanation, now I see the difference in semantic. You’ll want to send an email to give a heads-up to out of tree targets for this change.

Also, before landing the patch, ping Tom (thomas.stellard at amd.com, code owner of the AMDGPU), to see if the changes in AMDGPU also look good to him.

Cheers,
-Quentin


================
Comment at: llvm/lib/Target/AMDGPU/SIFoldOperands.cpp:178
@@ -177,2 +177,3 @@
 
-    if (!CanCommute || !TII->commuteInstruction(MI))
+    // FIXME: One of operands might be an Imm operand, and OpNo may refer
+    // to it after the call of commuteInstruction() below. Such situations
----------------
The comment is just fine without the FIXME in front of it.
Just remove it, i.e., I do not think there is anything to fix here in the end.


http://reviews.llvm.org/D11370