Re: Add support to trace comparison instructions and switch statements

["Dmitry Vyukov via gcc-patches" <gcc-patches@gcc.gnu.org>]

On Sun, Sep 3, 2017 at 12:01 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Sun, Sep 03, 2017 at 10:50:16AM +0200, Dmitry Vyukov wrote:
>> What we instrument in LLVM is _comparisons_ rather than control
>> structures. So that would be:
>>     _4 = x_8(D) == 98;
>> For example, result of the comparison can be stored into a bool struct
>> field, and then used in branching long time after. We still want to
>> intercept this comparison.
>
> Then we need to instrument not just GIMPLE_COND, which is the stmt
> where the comparison decides to which of the two basic block successors to
> jump, but also GIMPLE_ASSIGN with tcc_comparison class
> gimple_assign_rhs_code (the comparison above), and maybe also
> GIMPLE_ASSIGN with COND_EXPR comparison code (that is say
>   _4 = x_1 == y_2 ? 23 : _3;
> ).
>
>> > Perhaps for -fsanitize-coverage= it might be a good idea to force
>> > LOGICAL_OP_NON_SHORT_CIRCUIT/BRANCH_COST or whatever affects GIMPLE
>> > decisions mentioned above so that the IL is closer to what the user wrote.
>>
>> If we recurse down to comparison operations and instrument them, this
>> will not be so important, right?
>
> Well, if you just handle tcc_comparison GIMPLE_ASSIGN and not GIMPLE_COND,
> then you don't handle many comparisons from the source code.  And if you
> handle both, some of the GIMPLE_CONDs might be just artificial comparisons.
> By pretending small branch cost for the tracing case you get fewer
> artificial comparisons.


Are these artificial comparisons on BOOLEAN_TYPE? I think BOOLEAN_TYPE
needs to be ignored entirely, there is just like 2 combinations of
possible values.
If not, then what it is? Is it a dup of previous comparisons?

I am not saying these modes should not be enabled. You know much
better. I just wanted to point that that integer comparisons is what
we should be handling.

Your example:

  _1 = x_8(D) == 21;
  _2 = x_8(D) == 64;
  _3 = _1 | _2;
  if (_3 != 0)

raises another point. Most likely we don't want to see speculative
comparisons. At least not yet (we will see them once we get through
the first comparison). So that may be another reason to enable these
modes (make compiler stick closer to original code).