[llvm-commits] Add support for shl, lshr, and ashr instructions

[Chad Rosier]

On Aug 2, 2012, at 1:33 AM, Jush Lu <jush.msn at gmail.com> wrote:

> 
> Hi,
> 
> Attachment is new patch, basically I use the test cases you gave in last email, I just add few lines for FileCheck.
> 
> I think falling back to selection DAG isel is really better way when the shift amount is zero or greater than the width of the value type, so I modified code for this.
> 
> I also modified code as Jim’ suggestion. 
> 
> Thanks,
> Jush 
> 
> 
> On Thu, Aug 2, 2012 at 2:57 AM, Jim Grosbach <grosbach at apple.com> wrote:
> 
> On Aug 1, 2012, at 11:18 AM, Chad Rosier <mcrosier at apple.com> wrote:
> 
>> Jush,
>> In the case that the shift amount is zero or greater then the width of the value type, should we just fall back to selection DAG isel to ensure we have consistent behavior (in the event that selection DAG isel changes)?  Perhaps someone else could comment on this.
> 
> 
> This seems reasonable to me.
> 
> Additional minor thing:
> 
> +  if (isa<ConstantInt>(Src2Value)) {
> +    Opc = ARM::MOVsi;
> +    const ConstantInt *CI = cast<ConstantInt>(Src2Value);
> 
> This can be simplified to:
> if (const ConstantInt *CI = dyn_cast<ConstantInt>(Src2Value)) {
>   Opc = ARM::MOVsi;
> 
> 
> -Jim
> 
>> 
>> My only other suggestion would be to simplify the test cases.  For example,
>> 
>> define i32 @shl() nounwind ssp {
>> entry:
>>   %shl = shl i32 -1, 2
>>   ret i32 %shl
>> }
>> 
>> define i32 @shl_reg(i32 %src1, i32 %src2) nounwind ssp {
>> entry:
>>   %shl = shl i32 %src1, %src2
>>   ret i32 %shl
>> }
>> 
>> define i32 @lshr() nounwind ssp {
>> entry:
>>   %lshr = lshr i32 -1, 2
>>   ret i32 %lshr
>> }
>> 
>> define i32 @lshr_reg(i32 %src1, i32 %src2) nounwind ssp {
>> entry:
>>   %lshr = lshr i32 %src1, %src2
>>   ret i32 %lshr
>> }
>> 
>> define i32 @ashr() nounwind ssp {
>> entry:
>>   %ashr = ashr i32 -1, 2
>>   ret i32 %ashr
>> }
>> 
>> define i32 @ashr_reg(i32 %src1, i32 %src2) nounwind ssp {
>> entry:
>>   %ashr = ashr i32 %src1, %src2
>>   ret i32 %ashr
>> }
>> 
>> Basically, I ran mem2reg on your test cases… :)  
>> 
>> While I realize this isn't the type of code fast-isel is going to generate, this is much more concise in terms of what you're trying to test (and that's all we care about).
>> 
>> Please apply after updating the test cases.  If anyone disagrees with the handing of the zero/32 shift we can handle that as a post commit; I'm ok with leaving as is..
>> 
>>  Chad
>> 
>> 
>> On Jul 31, 2012, at 11:05 PM, Jush Lu wrote:
>> 
>>> Hi
>>> 
>>> This patch makes ARMFastISel able to handle shl, lshr, and ashr instructions, please help me review it, thanks.
>>> 
>>> Jush <fast-isel-shifter.patch>
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> 
> <fast-isel-shifter.patch>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120802/a7c1947c/attachment.html>