Stam Markianos-Wright <stam.markianos-wri...@arm.com> writes: > On 1/29/20 12:42 PM, Richard Sandiford wrote: >> Stam Markianos-Wright <stam.markianos-wri...@arm.com> writes: >>> Hi all, >>> >>> This fixes: >>> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93300 >>> >>> Genmodes.c was generating the "wider_mode" chain as follows: >>> >>> HF -> BF -> SF - > DF -> TF -> VOID >>> >>> This caused issues in some rare cases where conversion between modes >>> was needed, such as the above PR93300 where BFmode was being picked up >>> as a valid mode for: >>> >>> optabs.c:prepare_float_lib_cmp >>> >>> which then led to the ICE at expr.c:convert_mode_scalar. > > Hi Richard, > >> >> Can you go into more details about why this chain was a problem? >> Naively, it's the one I'd have expected: HF should certainly have >> priority over BF, > > Is that because functionally it looks like genmodes puts things in reverse > alphabetical order if all else is equal? (If I'm reading the comment about > MODE_RANDOM, MODE_CC correctly) > >> but BF coming before SF doesn't seem unusual in itself. >> >> I'm not saying the patch is wrong. It just wasn't clear why it was >> right either. >> > Yes, I see what you mean. I'll go through my thought process here: > > In investigating the ICE PR93300 I found that the diversion from pre-bf16 > behaviour was specifically at `optabs.c:prepare_float_lib_cmp`, where a > `FOR_EACH_MODE_FROM (mode, orig_mode)` is used to then go off and generate > library calls for conversions. > > This was then being caught further down by the gcc_assert at expr.c:325 where > GET_MODE_PRECISION (from_mode) was equal to GET_MODE_PRECISION (to_mode) > because > it was trying to emit a HF->BF conversion libcall as `bl __extendhfbf2` > (which > is what happened if i removed the gcc_assert at expr.c:325) > > With BFmode being a target-defined mode, I didn't want to add something like > `if > (mode != BFmode)` to specifically exclude BFmode from being selected for > this. > (and there's nothing different between HFmode and BFmode here to allow me to > make this distinction?) > > Also I couldn't find anywhere where the target back-end is not consulted for > a > "is this supported: yes/no" between the `FOR_EACH_MODE_FROM` loop and the > libcall being created later on as __extendhfbf2.
Yeah, prepare_float_lib_cmp just checks for libfuncs rather than calling target hooks directly. The libfuncs themselves are under the control of the target though. By default we assume all float modes have associated libfuncs. It's then up to the target to remove functions that don't exist (or redirect them to other functions). So I think we need to remove BFmode libfuncs in arm_init_libfuncs in the same way as we currently do for HFmode. I guess we should also nullify the conversion libfuncs for BFmode, not just the arithmetic and comparison ones. Thanks, Richard > Finally, because we really don't want __bf16 to be in the same "conversion > rank" > as standard float modes for things like automatic promotion, this seemed like > a > reasonable solution to that problem :) > > Let me know of your thoughts! > > Cheers, > Stam
