feedable <[email protected]> writes: > On 09/09/2026 23:14, Richard Sandiford wrote: >> feedable <[email protected]> writes: >>> On 09/09/2026 21:02, Richard Sandiford wrote: >>>> FWIW, this is what I had in mind. Bootstrapped & regression-tested on >>>> x86_64-linux-gnu. Also tested by cross-building sh-linux-gnu. WDYT? >>>> >>>> Richard >>> Irrespective of the `auto_vec` state, I think this is a positive change. >>> However, do note that at least aarch64 also does the same trick with >>> copying `recog_data` in its early-ra, and other backends may also do >>> that. >> Are you sure about early-ra? It swaps operands around, but it doesn't >> seem to do a full copy AFAICT. At least, the change below didn't seem >> to trip over it. > Hmm, upon further inspection, I think I am misremembering. There was a > subtlety regarding interplay between extract_insn and recog, and I > misremembered that as being one regarding recog_data_d itself.
Ah, ok, thanks. >>> If we are to proceed with this change, I would advise deleting >>> copy&move construction and assignment for `recog_data_d` to explicitly >>> fail compilation on the uses that remain. IIRC there should be a couple >>> more copies in IRA&LRA, so this should reveal those too. >> Ah yeah, good point. Doing that shows that I missed the case in >> reload.cc. >> >> Before posting the patch, I did try grepping for recog_data_d, for >> assignments to recog_data (in case of auto), and various other things, >> but I couldn't see anything in other backends. I suppose I should do >> a "build all targets" to check though. (I missed the reload.cc one >> because I'd misread it as being part of the recog* stuff.) >> >> Deleting the constructors means that I need to do something about: >> >> memset (&recog_data, 0, sizeof (recog_data)); >> >> in varasm.cc:assemble_asm (added by r15-5932-gca4d6285974817). >> TBH I'm not sure why that's necessary. Maybe it was just included >> to be conservative? I see that in your series you dropped the >> memsetting here, but I suppose your series also adds more runtime >> checking thanks to using vec operator[] over plain arrays. > Well, my patch doesn't strictly drop that memsetting, it replaces that > with a call to init. That member function itself does reset all members > to zeroes, as well as sets the operand and dup count. That said, I also > think that the memset there is not necessary. But I also think that you > can still do memsets even if you delete ctors, so remove that or not, > either is fine with me. I should have said, but the issue with the memset is that it trips -Wclass-memaccess, which becomes an error during bootstrap. Richard
