[PATCH] ARMv8 IfConversion must skip narrow instructions that a) define CPSR and b) wouldn't affect CPSR in an IT block

[Weiming Zhao]

Hi Artyom,

I understand that your patch is targeting the first issue. I'm just not so sure about the second issue. 
Anyway, it looks good to me.

Thanks,
Weiming

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


-----Original Message-----
From: Artyom Skrobov [mailto:Artyom.Skrobov at arm.com] 
Sent: Tuesday, February 25, 2014 11:28 AM
To: weimingz at codeaurora.org
Cc: 'llvm-commits'; 'Tim Northover'
Subject: RE: [PATCH] ARMv8 IfConversion must skip narrow instructions that a) define CPSR and b) wouldn't affect CPSR in an IT block

Weiming, my point was that there are two problems with ARMv8 IfConversion: that it can convert some instructions that shouldn't be converted, and that wrong mnemonics are generated for some instructions that are rightfully converted. My patch only deals with the first problem, and your test-case only triggered the second problem; but both problems are equally valid.

Do you want to run some tests before committing this patch, or does it already look convincing enough? :-)

As I replied to Tim in an earlier mail, this change only applies to ARMv8 AArch32, because in ARMv7 the SizeReduction step doesn't run until after the IfConversion, and therefore IfConversion in v7 cannot encounter these narrow Thumb instructions.



-----Original Message-----
From: Weiming Zhao [mailto:weimingz at codeaurora.org]
Sent: 25 February 2014 19:16
To: 'Tim Northover'; Artyom Skrobov
Cc: 'llvm-commits'
Subject: RE: [PATCH] ARMv8 IfConversion must skip narrow instructions that a) define CPSR and b) wouldn't affect CPSR in an IT block

Hi Artyom,

Sorry for late reply. I read the code and LGTM. But I haven't applied the code and run some tests.
For the original bug, I am not fully convinced that it is due to emitting a wrong assembly mnemonic because I got run-time mismatch with integrated-as.

I think the functionality is for aarch32 only.

Thanks,
Weiming

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

-----Original Message-----
From: Tim Northover [mailto:t.p.northover at gmail.com]
Sent: Tuesday, February 25, 2014 8:38 AM
To: Artyom Skrobov
Cc: llvm-commits; Weiming Zhao
Subject: Re: [PATCH] ARMv8 IfConversion must skip narrow instructions that a) define CPSR and b) wouldn't affect CPSR in an IT block

Hi Artyom,

> Therefore, when IfConversion applies the new ARMv8 restrictions, it 
> must not embed such an instruction into an IT block *whenever CPSR is 
> live after the instruction*.

Doesn't this apply on v7 as well? If so, that v8 function is the wrong place to apply the fix.

Cheers.

Tim.