[llvm-commits] [llvm] r171136 - /llvm/trunk/lib/Target/X86/X86InstrArithmetic.td

[Jakob Stoklund Olesen]

On Jan 2, 2013, at 11:44 AM, Craig Topper <craig.topper at gmail.com> wrote:

> OK. Should we more explicitly set "hasSideEffects = 1" then so that its not subtly happening due to lack of a pattern?

Yes, I think that is best. Ideally, all targets should work with guessInstructionProperties = 0. I would like to make that the default.

You can add the test case to test/CodeGen/X86/early-ifcvt.ll 

Thanks,
/jakob


> 
> On Wed, Jan 2, 2013 at 11:11 AM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
> 
> On Jan 2, 2013, at 11:04 AM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
> 
> >
> > On Dec 26, 2012, at 7:01 PM, Craig Topper <craig.topper at gmail.com> wrote:
> >
> >> Author: ctopper
> >> Date: Wed Dec 26 21:01:18 2012
> >> New Revision: 171136
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=171136&view=rev
> >> Log:
> >> Mark the divide instructions as hasSideEffects=0.
> >
> > Hi Craig,
> >
> > I don't think this is safe because div instructions can trap when the divisor is 0, and that means it is not safe to move them between basic blocks. See also llvm::isSafeToSpeculativelyExecute().
> 
> For example:
> 
> $ cat unsafe-div.c
> int f(int a, int b) {
>   return b ? a/b : a;
> }
> 
> $ Release+Asserts/bin/clang -O -o- -S unsafe-div.c -mllvm -x86-early-ifcvt -mllvm -stress-early-ifcvt
>         .section        __TEXT,__text,regular,pure_instructions
>         .globl  _f
>         .align  4, 0x90
> _f:                                     ## @f
>         .cfi_startproc
> ## BB#0:                                ## %entry
>         pushq   %rbp
> Ltmp2:
>         .cfi_def_cfa_offset 16
> Ltmp3:
>         .cfi_offset %rbp, -16
>         movq    %rsp, %rbp
> Ltmp4:
>         .cfi_def_cfa_register %rbp
>         movl    %edi, %eax
>         cltd
>         idivl   %esi
>         testl   %esi, %esi
>         cmovel  %edi, %eax
>         popq    %rbp
>         ret
>         .cfi_endproc
> 
> 
> .subsections_via_symbols
> 
> 
> 
> 
> -- 
> ~Craig

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130102/d9dac95a/attachment.html>