Re: [ARM] PR 67591 ARM v8 Thumb IT blocks are deprecated

[Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>]

Hi Christophe,

On 07/09/16 21:05, Christophe Lyon wrote:
> Hi,
>
> The attached patch is a first part to solve PR 67591: it removes
> several occurrences of "IT blocks containing 32-bit Thumb
> instructions are deprecated in ARMv8" messages in the
> gcc/g++/libstdc++/fortran testsuites.
>
> It does not remove them all yet. This patch only modifies the
> *cmp_and, *cmp_ior, *ior_scc_scc, *ior_scc_scc_cmp,
> *and_scc_scc and *and_scc_scc_cmp patterns.
> Additional work is required in sub_shiftsi etc, at least.
> I've started looking at these, but I decided I could already
> post this self-contained patch to check if this implementation
> is OK.
>
> Regarding *cmp_and and *cmp_ior patterns, the addition of the
> enabled_for_depr_it attribute is aggressive in the sense that it keeps
> only the alternatives with 'l' and 'Py' constraints, while in some
> cases the constraints could be relaxed. Indeed, these 2 patterns can
> swap their input comparisons, meaning that any of them can be emitted
> in the IT-block, and is thus subject to the ARMv8 deprecation.
> The generated code is possibly suboptimal in the cases where the
> operands are not swapped, since 'r' could be used.
>
> Cross-tested on arm-none-linux-gnueabihf with -mthumb/-march=armv8-a
> and --with-cpu=cortex-a57 --with-mode=thumb, showing only improvements:
> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/239850-depr-it-4/report-build-info.html
>
> Bootstrapped OK on armv8l HW.
>
> Is this OK?
>
> Thanks,
>
> Christophe

 (define_insn_and_split "*ior_scc_scc"
-  [(set (match_operand:SI 0 "s_register_operand" "=Ts")
+  [(set (match_operand:SI 0 "s_register_operand" "=Ts,Ts")
 	(ior:SI (match_operator:SI 3 "arm_comparison_operator"
-		 [(match_operand:SI 1 "s_register_operand" "r")
-		  (match_operand:SI 2 "arm_add_operand" "rIL")])
+		 [(match_operand:SI 1 "s_register_operand" "r,l")
+		  (match_operand:SI 2 "arm_add_operand" "rIL,lPy")])
 		(match_operator:SI 6 "arm_comparison_operator"
-		 [(match_operand:SI 4 "s_register_operand" "r")
-		  (match_operand:SI 5 "arm_add_operand" "rIL")])))
+		 [(match_operand:SI 4 "s_register_operand" "r,l")
+		  (match_operand:SI 5 "arm_add_operand" "rIL,lPy")])))

Can you please put the more restrictive alternatives (lPy) first?
Same with the other patterns your patch touches.
Ok with that change if a rebased testing run is ok.
Sorry for the delay in reviewing.

Thanks for improving this area!
Kyrill