On Mon, Mar 18, 2024 at 3:51 PM Segher Boessenkool <seg...@kernel.crashing.org> wrote: > > On Thu, Mar 07, 2024 at 11:46:54PM +0100, Uros Bizjak wrote: > > > Can't you just describe the dataflow then, without an unspec? An unspec > > > by definition does some (unspecified) operation on the data. > > > > Previously, it was defined as: > > > > (define_insn "*pushfl<mode>2" > > [(set (match_operand:W 0 "push_operand" "=<") > > (match_operand:W 1 "flags_reg_operand"))] > > > > But Wmode, AKA SI/DImode is not CCmode. And as said in my last > > message, nothing prevents current source code to try to update the CC > > register here. > > So you can use an unspec just to convert the flags reg to an integer? > To make an integer representation of flags reg contents.
Yes, this is correct. But please note the v3 patch, where the mode update is made at the correct location. Quote from the patch: Replace cc_use_loc with the entire new RTX only in case cc_use_loc satisfies COMPARISON_P predicate. Otherwise scan the entire cc_use_loc RTX for CC reg to be updated with a new mode. > Or is that what we started with here? The reason for the patch is that when CC reg is used outside comparison RTX, the combine tries to update CC reg mode where it is used after the combined instruction. This happens on extremely rare occasions, but when it happens, combine assumes that it is used exclusively in the comparison RTX and uses "SUBST (XEXP (*cc_use_loc, 0), ...);". XEXP (*cc_use_loc, 0) will segfault when CC reg is referred in a simple SET assignment, not only when it is referred in an UNSPEC. Please note that the comparison RTX is handled a few lines above, where my patch also fixes the "???" issue. Uros.
