[llvm-dev] [AArch64] bug in shrink-wrapping

[Eric Christopher via llvm-dev]

On Fri, Nov 20, 2015 at 11:57 AM Quentin Colombet via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> On Nov 20, 2015, at 9:55 AM, Arnaud A. de Grandmaison <
> arnaud.degrandmaison at arm.com> wrote:
>
>
>
> -----Original Message-----
> From: qcolombet at apple.com [mailto:qcolombet at apple.com
> <qcolombet at apple.com>]
> Sent: 20 November 2015 18:07
> To: Arnaud De Grandmaison
> Cc: haicheng at codeaurora.org; llvm-dev at lists.llvm.org
> Subject: Re: [llvm-dev] [AArch64] bug in shrink-wrapping
>
> Hi Arnaud,
>
> Thanks for following up with that and sorry for the breakage.
>
> Couple of comments:
>   MachineLoopInfo *MLI;
> +  RegScavenger *RS;
>
> Would it make sense to use a unique_ptr here?
> That should eliminate the need of having explicit deletes.
>
>
> Using unique_ptr was my first attempt at fixing the memory leak :)
>
> I do not think you can, unless there is a way to "reset" (or create for the
> first time) the register scavenger for each function which requires it.
>
> The logical place where to put the unique_ptr would be in the
> runOnMachineFunction (and not as a class member) because this method has
> multiple exit paths, but the scavenger has to be passed around several
> calls
> then.
>
>
> Looks like you have a better hand on this than me :).
>
>
>
> +; RUN: llc -mtriple=aarch64-linux-gnu -o - %s
>
> Add -enable-shrink-wrap=true and a second RUN line with -enable-shrink-
> wrap=false.
> Then add check lines for both to ensure shrink-wrapping is doing the right
> thing.
>
>
> I was checking here for the crash only, so not having a crash is a
> successfully passing test. I should maybe improve the comment.
>
> If I understand you well, you want to additionally check shrink-wrapping is
> doing the right thing as it seems there was a coverage hole there.
>
> I would suggest that this is added as a follow-up patch, as this is for now
> breaking internal bots and it may take a bit of time for me to understand
>
> what are the actual expectations from shrink-wrap.
>
>
> Fair enough, please proceed!
>

FWIW I'm running into this as well :)


-eric


>
> Thanks,
> Q.
>
>
>
> +  %0 = load i32, i32* @g1, align 4
> Please use opt -instnamer to get rid of the numbered variables. Those are
>
> a
>
> pain when updating the tests :).
>
>
> Sure, will do.
>
>
> Other than that LGTM!
>
> Cheers,
> -Quentin
>
> On Nov 20, 2015, at 6:31 AM, Arnaud A. de Grandmaison
>
> <arnaud.degrandmaison at arm.com> wrote:
>
>
> +CC llvm-dev
>
> -----Original Message-----
> From: Arnaud A. de Grandmaison
>
> [mailto:arnaud.degrandmaison at arm.com <arnaud.degrandmaison at arm.com>]
>
> Sent: 20 November 2015 15:28
> To: 'qcolombet at apple.com'
> Cc: 'haicheng at codeaurora.org'
> Subject: RE: [llvm-dev] [AArch64] bug in shrink-wrapping
>
> Now with memory leak addressed.
>
> Cheers,
> Arnaud
>
> -----Original Message-----
> From: Arnaud A. de Grandmaison
>
> [mailto:arnaud.degrandmaison at arm.com <arnaud.degrandmaison at arm.com>]
>
> Sent: 20 November 2015 14:42
> To: 'qcolombet at apple.com'
> Cc: 'haicheng at codeaurora.org'
> Subject: RE: [llvm-dev] [AArch64] bug in shrink-wrapping
>
> There is a memory leak in my previous patch, let me fix it.
>
> Cheers,
> Arnaud
>
> -----Original Message-----
> From: Arnaud A. de Grandmaison
>
> [mailto:arnaud.degrandmaison at arm.com <arnaud.degrandmaison at arm.com>]
>
> Sent: 20 November 2015 12:49
> To: qcolombet at apple.com
> Cc: 'haicheng at codeaurora.org'
> Subject: RE: [llvm-dev] [AArch64] bug in shrink-wrapping
>
> Hi Quentin,
>
> I believe the attached patch fixes the issue. Can you review it ?
>
> Cheers,
> Arnaud
>
> -----Original Message-----
> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org
> <llvm-dev-bounces at lists.llvm.org>] On Behalf
> Of via llvm-dev
> Sent: 20 November 2015 05:37
> To: qcolombet at apple.com
> Cc: llvm-dev at lists.llvm.org
> Subject: [llvm-dev] [AArch64] bug in shrink-wrapping
>
> Hi Quentin,
>
> After shrink-wrapping was enabled as default on AArch64, llc has a
> seg fault when compiling the attached .ll file on AArch64.
>
> My command is
>
> llc -mcpu=cortex-a57 bug.ll
>
> Best,
>
> Haicheng
>
> <0001-ShrinkWrap-Teach-ShrinkWrap-to-handle-targets-requir.patch>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20151120/78dac0f5/attachment.html>