[PATCH] D111857: 【TwoAddressInstructionPass】 Put all new instructions into DistanceMap

Wed Oct 20 23:37:42 PDT 2021

pengfei added inline comments.

================
Comment at: llvm/test/CodeGen/X86/distancemap.mir:71
+    ; CHECK-NEXT: [[COPY6:%[0-9]+]]:gr16 = COPY [[COPY3]]
+    ; CHECK-NEXT: [[ADD16rr:%[0-9]+]]:gr16 = ADD16rr [[ADD16rr]], killed [[COPY4]], implicit-def $eflags
+    ; CHECK-NEXT: undef %11.sub_16bit:gr32 = COPY killed [[ADD16rr]]
----------------
Carrot wrote:
> pengfei wrote:
> > This does seem correct to me. The input in line 88 is
> > `%10:gr16 = ADD16rr killed %3:gr16($di), killed %5:gr16, ...`
> > But the output turns into add `[[COPY4]]($esi)`?
> The operands are commuted. This is the exact difference caused by this patch.
> 
> Function isProfitableToCommute calls noUseAfterLastDef with argument [[COPY3]] when processing this ADD instruction, DistanceMap is used in noUseAfterLastDef to decide MI's position.  When the LEA instruction was generated, several COPY and IMPLICIT_DEF instructions are generated. If they are not added to DistanceMap, noUseAfterLastDef misses the use of [[COPY3]] and returns wrong value. With this patch noUseAfterLastDef can correctly find the use of [[COPY3]] and returns correct value, causes isProfitableToCommute to return different commute decision.
Oh, I was misled by the name `COPY6`. The VR names are nonsense here. This should be bug for update script.
Unfortunately, this test doesn't reveal the intent of the patch either. The swapped `COPY3` and `COPY4` doesn't show the necessity, which is the same as other tests. I'd suggest to remove it due to the misleading output.
Besides, both `COPY3` and `COPY4` are used in LEA. The explanation here doesn't persuasive.
I'd like to keep the code as is until we see a functional failure. But I'm neutral :)

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

https://reviews.llvm.org/D111857