llvm dev - Nov 2013 - [LLVMdev] Unaligned load/store for callee-saved 128-bit registers

----- Original Message -----
> From: "Hal Finkel" <hfinkel at anl.gov>
> To: "Francois Pichet" <pichet2000 at gmail.com>
> Cc: "LLVM Developers Mailing List" <llvmdev at cs.uiuc.edu>
> Sent: Monday, November 18, 2013 2:45:53 PM
> Subject: Re: [LLVMdev] Unaligned load/store for callee-saved 128-bit
registers
> 
> ----- Original Message -----
> > From: "Francois Pichet" <pichet2000 at gmail.com>
> > To: "LLVM Developers Mailing List" <llvmdev at
cs.uiuc.edu>
> > Sent: Monday, November 18, 2013 2:26:30 PM
> > Subject: [LLVMdev] Unaligned load/store for callee-saved 128-bit
> > registers
> > 
> > 
> > 
> > On my (out-of-tree) target I have 16 128-bit registers.
> > Unaligned load/store are illegal. (must 16-bytes aligned)
> > 
> > 
> > 
> > 8 of those registers are defined as callee-saved and 8
> > caller-saved.
> > The default stack size is 4 bytes.
> > The target implements dynamic stack realign to make sure the stack
> > will always be aligned correctly when necessary.
> > 
> > 
> > Yet I am still getting unaligned load/store when running this test
> > case: http://pastie.org/8490604
> > 
> > 
> > The problem is in PEI::calculateCalleeSavedRegisters:
> > 
> > 
> > 
> > // We may not be able to satisfy the desired alignment
> > specification
> > of
> > // the TargetRegisterClass if the stack alignment is smaller. Use
> > the
> > // min.
> > Align = std::min(Align, StackAlign);
> > FrameIdx = MFI->CreateStackObject(RC->getSize(), Align, true);
> > 
> > 
> > This will create unaligned load/store for a callee-saved 128-bit
> > register on the frame slot because StackAlign is 4.
> > 
> > 
> > Adding a check for stack realignable or putting all the 128-bit
> > registers as caller-save will fix the problem.
> > 
> > if (!TFI->isStackRealignable()) <--- new line
> > Align = std::min(Align, StackAlign);
> > 
> > Is this a bug or am I missing something?
> > 
> 
> This looks like a bug. By default, isStackRealignable() always
> returns true (this default comes from the TargetFrameLowering
> constructor). I wonder, however, is this is not correctly
> implemented in some backends (X86RegisterInfo::canRealignStack, for
> example, is not completely trivial). Nadav, do you know how this
> works?
[Trying some other relevant people...]

Chad, Jakob: thoughts?

 -Hal
> 
>  -Hal
> 
> > 
> > Thanks,
> > Francois Pichet, Octasic.
> > 
> > _______________________________________________
> > LLVM Developers mailing list
> > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> > 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> 
-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory

BTW I managed to get around this problem by flagging all the 128-bit
registers as caller saved only.
On my system, vector registers are more likely to be used on leaf functions
anyway.



On Thu, Nov 21, 2013 at 3:24 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> ----- Original Message -----
> > From: "Hal Finkel" <hfinkel at anl.gov>
> > To: "Francois Pichet" <pichet2000 at gmail.com>
> > Cc: "LLVM Developers Mailing List" <llvmdev at
cs.uiuc.edu>
> > Sent: Monday, November 18, 2013 2:45:53 PM
> > Subject: Re: [LLVMdev] Unaligned load/store for callee-saved 128-bit
> registers
> >
> > ----- Original Message -----
> > > From: "Francois Pichet" <pichet2000 at gmail.com>
> > > To: "LLVM Developers Mailing List" <llvmdev at
cs.uiuc.edu>
> > > Sent: Monday, November 18, 2013 2:26:30 PM
> > > Subject: [LLVMdev] Unaligned load/store for callee-saved 128-bit
> > > registers
> > >
> > >
> > >
> > > On my (out-of-tree) target I have 16 128-bit registers.
> > > Unaligned load/store are illegal. (must 16-bytes aligned)
> > >
> > >
> > >
> > > 8 of those registers are defined as callee-saved and 8
> > > caller-saved.
> > > The default stack size is 4 bytes.
> > > The target implements dynamic stack realign to make sure the
stack
> > > will always be aligned correctly when necessary.
> > >
> > >
> > > Yet I am still getting unaligned load/store when running this
test
> > > case: http://pastie.org/8490604
> > >
> > >
> > > The problem is in PEI::calculateCalleeSavedRegisters:
> > >
> > >
> > >
> > > // We may not be able to satisfy the desired alignment
> > > specification
> > > of
> > > // the TargetRegisterClass if the stack alignment is smaller. Use
> > > the
> > > // min.
> > > Align = std::min(Align, StackAlign);
> > > FrameIdx = MFI->CreateStackObject(RC->getSize(), Align,
true);
> > >
> > >
> > > This will create unaligned load/store for a callee-saved 128-bit
> > > register on the frame slot because StackAlign is 4.
> > >
> > >
> > > Adding a check for stack realignable or putting all the 128-bit
> > > registers as caller-save will fix the problem.
> > >
> > > if (!TFI->isStackRealignable()) <--- new line
> > > Align = std::min(Align, StackAlign);
> > >
> > > Is this a bug or am I missing something?
> > >
> >
> > This looks like a bug. By default, isStackRealignable() always
> > returns true (this default comes from the TargetFrameLowering
> > constructor). I wonder, however, is this is not correctly
> > implemented in some backends (X86RegisterInfo::canRealignStack, for
> > example, is not completely trivial). Nadav, do you know how this
> > works?
>
> [Trying some other relevant people...]
>
> Chad, Jakob: thoughts?
>
>  -Hal
>
> >
> >  -Hal
> >
> > >
> > > Thanks,
> > > Francois Pichet, Octasic.
> > >
> > > _______________________________________________
> > > LLVM Developers mailing list
> > > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> > >
> >
> > --
> > Hal Finkel
> > Assistant Computational Scientist
> > Leadership Computing Facility
> > Argonne National Laboratory
> > _______________________________________________
> > LLVM Developers mailing list
> > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> >
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131121/b6f1011c/attachment.html>

----- Original Message -----
> From: "Francois Pichet" <pichet2000 at gmail.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "Chad Rosier" <mcrosier at codeaurora.org>, "Jakob
Stoklund Olesen" <jolesen at apple.com>, "LLVM Developers
Mailing
> List" <llvmdev at cs.uiuc.edu>
> Sent: Thursday, November 21, 2013 2:36:06 PM
> Subject: Re: [LLVMdev] Unaligned load/store for callee-saved 128-bit
registers
> 
> 
> BTW I managed to get around this problem by flagging all the 128-bit
> registers as caller saved only.
> 
> On my system, vector registers are more likely to be used on leaf
> functions anyway.
> 
> 
Sounds good; however, unless I'm wrong, this looks like a general problem
that needs to be fixed.

 -Hal
> 
> 
> 
> 
> On Thu, Nov 21, 2013 at 3:24 PM, Hal Finkel < hfinkel at anl.gov >
> wrote:
> 
> 
> 
> 
> ----- Original Message -----
> > From: "Hal Finkel" < hfinkel at anl.gov >
> > To: "Francois Pichet" < pichet2000 at gmail.com >
> > Cc: "LLVM Developers Mailing List" < llvmdev at
cs.uiuc.edu >
> > Sent: Monday, November 18, 2013 2:45:53 PM
> > Subject: Re: [LLVMdev] Unaligned load/store for callee-saved
> > 128-bit registers
> > 
> > ----- Original Message -----
> > > From: "Francois Pichet" < pichet2000 at gmail.com
>
> > > To: "LLVM Developers Mailing List" < llvmdev at
cs.uiuc.edu >
> > > Sent: Monday, November 18, 2013 2:26:30 PM
> > > Subject: [LLVMdev] Unaligned load/store for callee-saved 128-bit
> > > registers
> > > 
> > > 
> > > 
> > > On my (out-of-tree) target I have 16 128-bit registers.
> > > Unaligned load/store are illegal. (must 16-bytes aligned)
> > > 
> > > 
> > > 
> > > 8 of those registers are defined as callee-saved and 8
> > > caller-saved.
> > > The default stack size is 4 bytes.
> > > The target implements dynamic stack realign to make sure the
> > > stack
> > > will always be aligned correctly when necessary.
> > > 
> > > 
> > > Yet I am still getting unaligned load/store when running this
> > > test
> > > case: http://pastie.org/8490604
> > > 
> > > 
> > > The problem is in PEI::calculateCalleeSavedRegisters:
> > > 
> > > 
> > > 
> > > // We may not be able to satisfy the desired alignment
> > > specification
> > > of
> > > // the TargetRegisterClass if the stack alignment is smaller. Use
> > > the
> > > // min.
> > > Align = std::min(Align, StackAlign);
> > > FrameIdx = MFI->CreateStackObject(RC->getSize(), Align,
true);
> > > 
> > > 
> > > This will create unaligned load/store for a callee-saved 128-bit
> > > register on the frame slot because StackAlign is 4.
> > > 
> > > 
> > > Adding a check for stack realignable or putting all the 128-bit
> > > registers as caller-save will fix the problem.
> > > 
> > > if (!TFI->isStackRealignable()) <--- new line
> > > Align = std::min(Align, StackAlign);
> > > 
> > > Is this a bug or am I missing something?
> > > 
> > 
> > This looks like a bug. By default, isStackRealignable() always
> > returns true (this default comes from the TargetFrameLowering
> > constructor). I wonder, however, is this is not correctly
> > implemented in some backends (X86RegisterInfo::canRealignStack, for
> > example, is not completely trivial). Nadav, do you know how this
> > works?
> 
> [Trying some other relevant people...]
> 
> Chad, Jakob: thoughts?
> 
> -Hal
> 
> 
> 
> > 
> > -Hal
> > 
> > > 
> > > Thanks,
> > > Francois Pichet, Octasic.
> > > 
> > > _______________________________________________
> > > LLVM Developers mailing list
> > > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> > > 
> > 
> > --
> > Hal Finkel
> > Assistant Computational Scientist
> > Leadership Computing Facility
> > Argonne National Laboratory
> > _______________________________________________
> > LLVM Developers mailing list
> > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> > 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> 
> 
-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory