On Fri, Dec 2, 2016 at 8:58 PM, James Greenhalgh <james.greenha...@arm.com> wrote: > On Fri, Dec 02, 2016 at 05:00:05PM +0100, Bernd Schmidt wrote: >> With the i386 backend no longer double-counting the cost of a SET, >> the default implementation default_max_noce_ifcvt_seq_cost now >> provides too high a bound for if conversion, allowing very costly >> substitutions. >> >> The following patch fixes this by making an i386 variant of the >> hook, but first - James, do you recall how you arrived at the >> COSTS_N_INSNS(3) magic number? What happens on arm if you lower that >> in the default hook? > > Hi Bernd, > > Given the magic numbers in BRANCH_COST already, 3 was chosen to give a > resonable chance of allowing if-conversion of blocks containing multiple > sets. iWe need to do this because for AArch64 and x86 the conditional > move pattern will expand to two further patterns, each of which will get > a cost (before my cost model patches you would simply count the total > number of expansions you would make, so the multiplication by three is > to compensate) > > I wouldn't say there was much scientific to choosing between 2 and 3 > beyond looking at cases which worked on x86_64 and AArch64 before I > modified the cost model, and again after, and trying to maintain the > number of such cases which were if-converted. > > Setting this to 2 in the generic hook and forcing AArch64 to run with > a custom hook would be just as correct as this patch (though arguably > yours is the better Stage 3 patch as it has more limited scope).
Based on the above explanation, the patch is OK. Thanks, Uros.
