[PATCH] D30177: [ExecutionDepsFix] Recognize existing dep breaks

[Marina Yatsina via Phabricator via llvm-commits]

myatsina added inline comments.


================
Comment at: lib/Target/X86/X86InstrInfo.cpp:8271
+  case X86::SUB64rr:
+  case X86::XORPSrr:
+  case X86::XORPDrr:
----------------
According to the guide, xorps and xorpd cannot break dependencies on pentium4, so this information should be added too.


================
Comment at: test/CodeGen/X86/break-false-dep.ll:339
+; Make sure we recognize pre-existing dependency breaking instructions and
+; re-use them. In `fcmp ult double %x, 0.0`, the `0.0` constant gets
+; materialized as a vxorps
----------------
This assumption seems to be very fragile, vector zero initializer should also create a xor, and probably be more robust.
I'm starting to wonder if this test should be written in mir instead, then we would not be dependent on code gen optimizations that create xors and we could test all the additional dependency breaking instructions easily, not just xor.




================
Comment at: test/CodeGen/X86/break-false-dep.ll:349
+;AVX: vucomisd  [[XMM1]], %xmm0
+  %1 = fcmp ult double %x, 0.0
+  br i1 %1, label %main, label %fake
----------------
What happens for this function?:

define double @recognize_existing(i64 %arg) {

; Mark all regs as "used" and thus having same clearance
 tail call void asm sideeffect "", "~{xmm0},...,~{xmm15},~{dirflag},~{fpsr},~{flags}"()

%tmp1 = sitofp i64 %arg to double
%tmp2 = sitofp i64 %arg to double

%tmp3 = fadd double %tmp1, %tmp2
ret %tmp3
}

I expect a xor for some xmm will added before the first register.
Will we see additional xor before the second function?
Meaning, does this optimization take into account the xors we add to break dependency and thus re calculate their clearance?


https://reviews.llvm.org/D30177