On Fri, Jan 22, 2016 at 9:32 AM, Kyrill Tkachov <kyrylo.tkac...@foss.arm.com> wrote: > Hi Han, > > On 21/01/16 22:57, Han Shen wrote: >> >> Hi Kyrill, the patched gcc generates correct asm for me for the test >> case. (I'll then build the whole system to see if other it-block >> related bugs are gone too.) >> >> One short question, the newly generated RTL for >> x = x | <cond> (a) >> will be >> orr %0, %1, #1; it <cond>; mov%D2\\t%0, %1 (b) >> >> The cond in (a) should be the reverse of cond in(b), right? > > > yes, the first C-like expression is just some pseudocode. > I guess it would be better written as: > x = x | <comparison result>. > > Thanks for trying it out. > I bootstrapped and tested this patch on trunk and GCC 5. > I'll be doing the same on the 4.9 branch.
Ok everywhere. While you are here could you also audit the other patterns that we changed for restrict_it to check that this thinko isn't present anywhere else, please ? Ramana > > Kyrill > > >> Thanks for your quick fix. >> >> Han >> >> On Thu, Jan 21, 2016 at 9:51 AM, Kyrill Tkachov >> <kyrylo.tkac...@foss.arm.com> wrote: >>> >>> Hi all, >>> >>> In this wrong-code PR the pattern for performing >>> x = x | <cond> for -mrestrict-it is buggy and ends up writing 1 to the >>> result register rather than orring it. >>> >>> The offending pattern is *thumb2_ior_scc_strict_it. >>> My proposed solution is to rewrite it as a splitter, remove the >>> alternative for the case where operands[1] and 0 are the same reg >>> that emits the bogus: >>> it <cond>; mov<cond>%0, #1; it <cond>; orr<cond> %0, %1 >>> >>> to emit the RTL equivalent to: >>> orr %0, %1, #1; it <cond>; mov%D2\\t%0, %1 >>> while marking operand 0 as an earlyclobber operand so that it doesn't >>> get assigned the same register as operand 1. >>> >>> This way we avoid the wrong-code, make the sequence better (by >>> eliminating >>> the move of #1 into a register >>> and relaxing the constraints from 'l' to 'r' since only the register move >>> has to be conditional). >>> and still stay within the rules for arm_restrict_it. >>> >>> Bootstrapped and tested on arm-none-linux-gnueabihf configured >>> --with-arch=armv8-a and --with-mode=thumb. >>> >>> Ok for trunk, GCC 5 and 4.9? >>> >>> Han, can you please try this out to see if it solves the problem on your >>> end >>> as well? >>> >>> Thanks, >>> Kyrill >>> >>> 2016-01-21 Kyrylo Tkachov <kyrylo.tkac...@arm.com> >>> >>> PR target/69403 >>> * config/arm/thumb2.md (*thumb2_ior_scc_strict_it): Convert to >>> define_insn_and_split. Ensure operands[1] and operands[0] do not >>> get assigned the same register. >>> >>> 2016-01-21 Kyrylo Tkachov <kyrylo.tkac...@arm.com> >>> >>> PR target/69403 >>> * gcc.c-torture/execute/pr69403.c: New test. >> >> >> >
