llvm dev - Oct 2008 - [LLVMdev] Target description flags for instructions which may trap

What are the correct target description side effect flags for 
instructions which may trap (e.g. divide / remainder)? The divide 
instruction in my backend currently has no flags set. I've enabled the 
MachineLICM pass and it's causing a miscompilation by hoisting a divide 
by zero instruction out of the loop. Clearly this pass needs to be made 
aware that this is not safe. The current test in the MachineLICM is as 
follows:

  // Ignore stuff that we obviously can't hoist.
  if (TID.mayStore() || TID.isCall() || TID.isReturn() || TID.isBranch() ||
      TID.hasUnmodeledSideEffects())
    return false;

Setting hasSideEffects = 1 seems to work, but I'm not sure if that's the
intended use of this flag. I notice that divide / remainder instructions 
for other architectures are not marked in this way. Also it is also 
pessimistic: if the divisor is know to be non zero then the instruction 
couldn't trap and would be safe to motion.

Richard Osborne | XMOS
http://www.xmos.com

On Oct 30, 2008, at 7:08 AM, Richard Osborne wrote:
> What are the correct target description side effect flags for
> instructions which may trap (e.g. divide / remainder)? The divide
> instruction in my backend currently has no flags set. I've enabled the
> MachineLICM pass and it's causing a miscompilation by hoisting a  
> divide
> by zero instruction out of the loop. Clearly this pass needs to be  
> made
> aware that this is not safe. The current test in the MachineLICM is as
> follows:
>
>  // Ignore stuff that we obviously can't hoist.
>  if (TID.mayStore() || TID.isCall() || TID.isReturn() ||  
> TID.isBranch() ||
>      TID.hasUnmodeledSideEffects())
>    return false;
>
> Setting hasSideEffects = 1 seems to work, but I'm not sure if
that's
> the
> intended use of this flag. I notice that divide / remainder  
> instructions
> for other architectures are not marked in this way. Also it is also
> pessimistic: if the divisor is know to be non zero then the  
> instruction
> couldn't trap and would be safe to motion.
All such side effects that are not explicitly modeled fall under the  
hasSideEffects bucket. It's something you can use now.

That said, there has been a lot of interest in enhancing LLVM with  
modeling trapping instructions. You should do a search on llvmdev.

Evan
>
>
> Richard Osborne | XMOS
> http://www.xmos.com
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

I think hasSideEffects is appropriate here.  MachineLICM isn't intended
to be able to hoist every possible loop-invariant instruction.  It's  
only
intended to be able to hoist invariant instructions that aren't  
exposed to
the main LLVM-IR-level LICM pass, such as constant-pool loads.

I don't think the LLVM-IR-level LICM pass is currently smart enough to
hoist divides if it can be proven that the divisor is non-zero.  But if
someone were looking to implement this feature, the LLVM-IR-level
LICM would be the place to do it, rather than MachineLICM.

I suspect the reason that other targets don't currently use  
hasSideEffects
on their divide instructions is just that they haven't had to yet.
MachineLICM is yet not enabled by default on mainline.

Dan

On Oct 30, 2008, at 7:08 AM, Richard Osborne wrote:
> What are the correct target description side effect flags for
> instructions which may trap (e.g. divide / remainder)? The divide
> instruction in my backend currently has no flags set. I've enabled the
> MachineLICM pass and it's causing a miscompilation by hoisting a  
> divide
> by zero instruction out of the loop. Clearly this pass needs to be  
> made
> aware that this is not safe. The current test in the MachineLICM is as
> follows:
>
>  // Ignore stuff that we obviously can't hoist.
>  if (TID.mayStore() || TID.isCall() || TID.isReturn() ||  
> TID.isBranch() ||
>      TID.hasUnmodeledSideEffects())
>    return false;
>
> Setting hasSideEffects = 1 seems to work, but I'm not sure if
that's
> the
> intended use of this flag. I notice that divide / remainder  
> instructions
> for other architectures are not marked in this way. Also it is also
> pessimistic: if the divisor is know to be non zero then the  
> instruction
> couldn't trap and would be safe to motion.
>
> Richard Osborne | XMOS
> http://www.xmos.com
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev