On Tue, Aug 12, 2014 at 12:31 PM, Kyrill Tkachov <kyrylo.tkac...@arm.com> wrote: > > On 12/08/14 10:39, Richard Biener wrote: >> >> On Mon, Aug 11, 2014 at 9:56 PM, Jeff Law <l...@redhat.com> wrote: >>> >>> On 08/11/14 07:41, Kyrill Tkachov wrote: >>>> >>>> >>>> I haven't been able to get combine to match the comparison+xor+neg+plus >>>> RTL and it seems like it would be just a workaround to undo the >>>> tree-level transformation. >>> >>> Yea, it'd just be a workaround, but it's probably the easiest way to deal >>> with this problem. Can you describe in further detail why you weren't >>> able >>> to get this to work? >> >> Too many instructions to combine I guess. You might want to add >> intermediate "combine" insn-and-splits. If that's still a no-go then >> read on. > > > From the combine dump I can see that it tried to combine up to: > (set (reg/i:SI 0 x0) > (plus:SI (xor:SI (neg:SI (reg:SI 84 [ D.2565 ])) > (reg:SI 73 [ D.2564 ])) > (reg:SI 84 [ D.2565 ]))) > > What I need is for that reg 84 to be the result of the comparison, something > like: > (ne (cc_reg) (const_int 0)) which I couldn't get combine to shove in there. > > > On the other hand, I did manage to write a peephole2 that detected the > sequence of compare+neg+xor+plus and transformed it into the if_then_else > form that our current conditional negation pattern has, although I'm not > sure how flexible this is. > > >> >> OTOH a suitable place to "undo" would be smarter RTL expansion >> that detects this pattern (and hope for TER to still apply - thus >> no CSE opportunities going in your way). For your testcase TER >> allows >> >> r_5 replace with --> r_5 = (int) _4; >> >> _8 replace with --> _8 = _4 != 0; >> >> _10 replace with --> _10 = -_9; >> >> _11 replace with --> _11 = _10 ^ r_5; >> >> _12 replace with --> _12 = _11 + _9; >> >> which unfortunately already "breaks" because of the multi-use _9 >> and _4. >> >> Now - the way out here is a GIMPLE pass right before expansion >> that performs pattern detection and generates target specific code >> suitable >> for optimal expand. Fortunately we already have that with >> pass_optimize_widening_mul (which does widen-mul and fma detection). > > > Maybe we can piggyback on this pass then? (probably renaming it to something > more generic in the process...) > > >> >> This is the place where you'd deal with such pattern, generating >> a suitable compound operation (or two, dependent on expansion >> and insn pattern details). This of course usually requires new >> tree codes or internal functions. For more arcane stuff like >> your conditional negate I'd prefer an internal function. > > > What is an internal function in this context? Is that like a target-specific > builtin? > > >> >> Of course you then have to add an optab or a target hook to >> do custom internal function expansion. I suppose for internal >> functions a target hook would be nicer than a generic optab? > > > So something like TARGET_EXPAND_CONDITIONAL_NEGATION that gets called from > the aforementioned GIMPLE pass when it finds a an appropriate sequence of > compare+neg+xor+plus ?
More like TARGET_CAN_EXPAND_IFN with the new GIMPLE internal-fn and a TARGET_EXPAND_IFN to actually perform the expansion. Richard. > Thanks for the pointers, > Kyrill > > >> >> Thanks, >> Richard. >> >> >> >>>> What is the most acceptable way of disabling this transformation for a >>>> target that has a conditional negation instruction? >>> >>> In general, we don't want target dependencies in the gimple/ssa >>> optimizers. >>> >>> Jeff > > >
