PATCH. Fix ARM Leapcrel definition to make it loop hoistable

[Jim Grosbach]

On Oct 14, 2013, at 11:20 AM, Yin Ma <yinma at codeaurora.org> wrote:

> Hi Tim,
> 
> It is difficult for me to create a test now.  Sorry about this.
> With this patch, I have run all validations and tests, all passed.
> 
> the ADR and LEApcrel are paired instructions to  handle pc-relative address.
> 
> ADR has not side effect. If we look at the Definition of  LEApcrel. It has
> much 
> similar definition like ADR. It should be marked as no side effect. 
> 

How does that answer Tim’s question? Those were added to solve a very real bug in the compiler. You need to address why this patch doesn’t reintroduce that bug.

-Jim

> Thanks,
> 
> Yin 
> 
> 
> -----Original Message-----
> From: Tim Northover [mailto:t.p.northover at gmail.com] 
> Sent: Monday, October 07, 2013 12:15 PM
> To: Yin Ma
> Cc: llvm-commits
> Subject: Re: PATCH. Fix ARM Leapcrel definition to make it loop hoistable
> 
> Hi Yin,
> 
>> This patch enables LEAPCREL to be moved out of a loop because it is 
>> currently marked as hasSideEffects in .td file. Actually, it is not. 
>> By reading the comment above this instruction. It should not be marked 
>> hasSideEffects from the very beginning.
> 
> Could you explain why Jakob's comment in r162603's commit message (when he
> added the "hasSideEffects = 1" line) is no longer valid?
> 
> Also, as usual, tests would be good.
> 
> Cheers.
> 
> Tim.
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits