On Wed, Jul 10, 2024 at 11:41 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > > > So I wanted to keep the nocsr check a little bit more permissive. > > > > > > > Also, instead of doing that extra nocsr offset check in > > > > do_misc_fixups(), why don't we just reset all no_csr patterns within a > > > > subprogram *if we find a single violation*. Compiler shouldn't ever > > > > emit such code, right? So let's be strict and fallback to not > > > > recognizing nocsr. no. > > > > And then we won't need that extra check in do_misc_fixups() because we > > > > eagerly unset no_csr flag and will never hit that piece of logic in > > > > patching. > > > > > > I can do that, but the detector pass would have to be two pass: > > > - on the first pass, find the nocsr_stack_off, add candidate insn marks; > > > - on the second pass, remove marks from insns with wrong stack access offset. > > > > It's not really a second pass, it's part of normal validation. > > check_nocsr_stack_contract() will detect this and will do, yes, pass > > over all instructions of a subprogram to unmark them. > > - on the first pass true .nocsr_stack_off is not yet known, > so .nocsr_pattern is set optimistically; > - on the second pass .nocsr_stack_off is already known, > so .nocsr_pattern can be removed from spills/fills outside the range; > - check_nocsr_stack_contract() does not need to scan full sub-program > on each violation, it can set a flag disabling nocsr in subprogram info. I really don't like where this discussion is going. Keep the current algo it's simple enough and matches what the compiler will generate. Obscure cases of users manually writing such patterns in asm are out of scope. Do not complicate the verifier for them.
