On Wed, Jul 10, 2024 at 12:07 PM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > 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. I agree with your arguments. > > > > > > 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. That's what I'm pushing for as well.
