Kewen: On 5/21/24 20:05, Kewen.Lin wrote: > Hi Carl, > > on 2024/5/22 08:13, Carl Love wrote: >> Kewen:
<snip> >>> Why did you place this in a section for ISA 3.1 (Power10)? It doesn't >>> really >>> require this support. The used instance VSEL_1TI and VSEL_1TI_UNS are >>> placed >>> in altivec stanza, so it looks that we should put it under the section >>> "PowerPC AltiVec Built-in Functions on ISA 2.05". And since it's an >>> extension >>> of @code{vec_sel} documented in the PVIPR, I prefer to just mention it's "an >>> extension of the @code{vec_sel} built-in documented in the PVIPR" and >>> omitting >>> the description to avoid possible slightly different wording. >> >> Honestly, at this point in time I don't remember why I put it there. It has >> been too long since I created the patch. That said, the test case requires >> Power 10 do to the comparison check using built-in vec_all_eq but that is >> another issue. >> The built-in generates the xxsel instruction that is an ISA 2.06 >> instruction. So, I would say it should to into the ISA 2.06 section. I >> moved it to the ISA 2.06 section. > > But the underlying implementation is: > > const vsq __builtin_altivec_vsel_1ti (vsq, vsq, vuq); > VSEL_1TI vector_select_v1ti {} > > const vuq __builtin_altivec_vsel_1ti_uns (vuq, vuq, vuq); > VSEL_1TI_UNS vector_select_v1ti_uns {} > > , it's under altivec stanza and can result with insn vsel (so not xxsel), > vsel is ISA 2.03, so I think ISA 2.05 better matches the implementation. OK, moved to ISA 2.05 > <snip> >> >> Sounds like there was some issue that you noticed on >> r14-10011-g6e62ede7aaccc6. The new version of >> print_i128 should be functionally equivalent but perhaps is "safer"? > > Thanks for checking! Looking into this more closely, I realized you didn't > apply the previously > adopted way for printing (the way used in > gcc.target/powerpc/builtins-6-p9-runnable.c), sorry for > the false alarm! So your supposed print_i128 is fine to me. OK, no problem. Will go with the original print_i128 function. Carl