[PATCH] D45995: [DAGCombiner] Set the right SDLoc on a newly-created zextload

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:8032
 
       ExtendSetCCUses(SetCCs, N0, ExtLoad, SDLoc(N), ISD::ZERO_EXTEND);
       // If the load value is used only by N, replace it via CombineTo N.
----------------
aprantl wrote:
> Out of curiosity, why doesn't this also get the `SDLoc(N0)`?
I'm not sure what SetCCUses are. It's possible that these need a different location. I can look into it as a follow-up.


================
Comment at: test/CodeGen/AArch64/arm64-aapcs.ll:31
+; CHECK-LABEL: test_stack_slots:
+; CHECK-DAG: ldr w[[ext1:[0-9]+]], [sp, #24]
+; CHECK-DAG: ldrh w[[ext2:[0-9]+]], [sp, #16]
----------------
aprantl wrote:
> So the order of these instructions is irrelevant?
It seems so, although it'd be nice if @ab could chime in about this.


================
Comment at: test/CodeGen/X86/fold-zext-trunc-dbginfo.ll:1
+; RUN: llc < %s -O0 -mtriple=x86_64-unknown-unknown -mcpu=x86-64 -stop-after livedebugvalues -o - | FileCheck %s
+
----------------
aprantl wrote:
> I'm not convinced if this huge test adds anything on top of all the other tests. Could you either remove it or replace it with a function that really only consists of the affected pattern?
There's an existing test I can repurpose.


https://reviews.llvm.org/D45995