On Wed, Oct 13, 2021 at 8:54 PM Alexandre Oliva <ol...@adacore.com> wrote:
>
> On Oct 12, 2021, Richard Biener <richard.guent...@gmail.com> wrote:
>
> > Are there any issues with respect to debugging when using such
> > asm()s?
>
> Not in this case. When creating short-lived copies for immediate use,
> like I do in the proposed patch, either the original value remains live
> in its original location and we use an actual copy, or the original
> value was dead, and we'll have a stmt/insn that visibly marks it as
> such, though the value actually remains there. The newly-added compare
> statements use these anonymous, temporary copies, so they're not
> relevant for debug information.
>
> Using asms could have effects on debug info and on optimizations in case
> their outputs *become* the location/value of the variable, i.e., as if
> in source code we did:
>
> asm ("" : "+g" (var));
>
> After this optimization barrier, the compiler wouldn't know any more
> anything it might have known before about the value held in the
> variable. And, if the variable is a gimple register, there would have
> to be a new debug bind stmt to bind the variable to its "new" value.
> (The debug machinery would assume the asm stmt modifies the value, and
> the output would thus overwrite the location with a value unrelated to
> the variable without the restated debug bind)
>
>
> The risk for debug info of introducing such asm stmts after conversion
> into SSA is that the debug binds wouldn't be added automatically, as
> they are when they're present in source code.
>
>
> >> Yeah, that would be another way to do it, but then it would have to be a
> >> lot trickier, given all the different ways in which compare-and-branch
> >> can be expressed in RTL.
>
> > Agreed, though it would be less disturbing to the early RTL pipeline
> > and RTL expansion.
>
> Is that a concern, though? It's not like such structures couldn't be
> present in source code, after all, so the RTL machinery has to be able
> to deal with them one way or another. For someone who wishes the
> compiler to introduce this hardening, the point in which it's added
> seems immaterial to me, as long as it remains all the way to the end.
>
>
> Now, if we had some kind of optimization barrier at the point of use, we
> would be able to save a copy in some cases. E.g., instead of:
>
> tmp = var;
> asm ("" : "+g" (tmp));
> if (tmp < cst) ...
> [reads from var]
>
> we could have:
>
> if (__noopt (var) < cst) ...
> [reads from var]
>
> and that would use var's value without taking an extra copy it, but also
> without enabling optimizations based on knowledge about the value.
>
> ISTM that introducing this sort of __noopt use would be a very
> significant undertaking. In RTL, a pseudo set to the original value of
> var, and that eventually resolves to the same location that holds that
> value, but marked in a way that prevents CSE, fwprop and whatnot (make
> it volatile?) would likely get 99% of the way there, but making pseudos
> to that end (and having such marks remain after reload) seems nontrivial
> to me.
Yeah, I think that eventually marking the operation we want to preserve
(with volatile?) would be the best way. On GIMPLE that's difficult,
it's easier on GENERIC (we can set TREE_THIS_VOLATILE on the
expression at least), and possibly also on RTL (where such flag
might already be a thing?). So when going that way doing the hardening
on RTL seems easier (if you want to catch all compares, if you want to
only catch compare + jump that has your mentioned issue of all the
different representations). Of course if volatile non-memory operations
are not a thing already on RTL (well, we _do_ have volatile UNSPECs ...)
then of course you still need to at least look at redundancy elimination
and expression combining passes to honor such flag.
Note that I did not look on the actual patch, I'm trying to see whether there's
some generic usefulness we can extract from the patchs requirement
which to me looks quite specific and given it's "hackish" implementation
way might not be the most important one to carry on trunk (I understand
that AdaCore will carry it in their compiler).
Thanks,
Richard.
>
> --
> Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/
> Free Software Activist GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts. Ask me about <https://stallmansupport.org>
