[PATCH] D45995: [DAGCombiner] Set the right SDLoc on a newly-created zextload (1/N)

[Vedant Kumar via Phabricator via llvm-commits]

vsk marked an inline comment as done.
vsk added a comment.

Some of the follow-up patches for issues identified in this review depend on the test case changes here.

Are there any outstanding issues left to address? I understand that the test case churn might be a blocking concern: if so, should I file a follow-up bug report against the scheduler, or should we introduce a workaround by preserving the old IROrder on new nodes?

In https://reviews.llvm.org/D45995#1082030, @aprantl wrote:

> So if I understand correctly the new instruction ordering in the assembler output is actually closer to the instruction ordering in the IR, since we are now setting the order metadata correctly in DAGCombine?


Yes, that's my read of what's happening. For example, in test/CodeGen/ARM/vector-load.ll, I think the add.w instruction corresponds to a gep, which should logically occur right before the store.



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7817
         TLI.isLoadExtLegal(ISD::SEXTLOAD, VT, MemVT)) {
       SDValue ExtLoad = DAG.getExtLoad(ISD::SEXTLOAD, DL, VT, LN0->getChain(),
                                        LN0->getBasePtr(), MemVT,
----------------
niravd wrote:
> Ditto
https://reviews.llvm.org/D46222


https://reviews.llvm.org/D45995