[PATCH] D75173: [Transform][MemCpyOpt] Add missing DebugLoc to %tmpbitcast

Vedant Kumar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 28 06:52:45 PST 2020


vsk added a comment.

In D75173#1897971 <https://reviews.llvm.org/D75173#1897971>, @Pierre-vh wrote:

> In D75173#1897944 <https://reviews.llvm.org/D75173#1897944>, @vsk wrote:
>
> > I’m not sure I understand why two run lines are needed, and neither are what I’d expect. The test has nothing to do with instcombine afaict, and running debugify /after/ memcpyopt is too late (that won’t test anything, as the real pass has already finished before we try attaching debug metadata).
> >
> > Istm you just need one run line: opt -debugify -memcpyopt -check-debugify
>
>
> You are right, I just misunderstood how opt worked (I didn't know the passes were run in order of appearance).
>  Would this better?
>
>   ; RUN: opt -instcombine -debugify -memcpyopt -check-debugify -S < %s 2>&1 | FileCheck %s
>  
>   ; CHECK: CheckModuleDebugify: PASS
>  
>   ; CHECK-LABEL: define {{.*}} @_Z3bar3Foo
>   ; CHECK: [[target:%.*]] = load i8*, i8** bitcast (%struct.Foo** @a to i8**), align 8, !dbg
>   ; CHECK: %tmpcast = bitcast i8* [[target]] to %struct.Foo*, !dbg
>


That looks perfect, except

> Note that I need `-instcombine` or else the test does not pass.

Hm, it looks like instcombine needs to massage the IR a bit before memcpyopt can "kick in". There's room in llvm for integration tests, but generally we try to make tests for transforms as narrow as possible (typically you don't want a change in instcombine breaking your test that's really for memcpyopt -- but sometimes you do). You can do that here with

opt -S -instcombine pr37967.ll > simplified.ll

Then your test should fully work with 'opt -debugify -memcpyopt -check-debugify'.


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

https://reviews.llvm.org/D75173





More information about the llvm-commits mailing list