Re: [patch][mudflap] Mudflap cleanup

[Richard Guenther <richard dot guenther at gmail dot com>]

On Fri, Jul 17, 2009 at 9:44 PM, Olatunji Ruwase<tjruwase@google.com> wrote:
> This cleanup makes it obvious as to how mudflap could be rewritten as
> a plugin. Below are the highlights of the patch.
>
> Mudflap now exports only two functions (mudflap_init,
> mudflap_finish_unit) to the rest of gcc.
>
> Also all but two uses of flag_mudflap have been eliminated from the
> rest of gcc, the remaining uses are for invoking the two exported
> functions.
>
> Another highlight of the patch is the elimination of decl marking
> (mf_mark(), mf_marked_p(), marked_trees).
> Mudflap used to mark decls to avoid
> ? ?(i) instrumenting synthetic decls ?and
> ? ?(ii) re instrumenting globals.
> Both requirements are now met by
> ? (i) explicitly identifying synthetic decls/ssa vars and
> ? (ii) instrumenting globals once at the end of compilation using
> varpool and constant_pool.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu. No regressions
> (including libmudflap testsuite).

Hm, what's this?

+  /* Enable pragma redefines. */
+  targetm.handle_pragma_redefine_extname = true;
+

+/* An SSA expression is eligible for instrumentation if
+   composed of at least one non-artificial subexpression/var.
+*/

Closing comment on the previous line.

+    if (ops[i] == NULL_TREE) {
+      continue;
+    }

no braces around single-stmt groups (also elsewhere).

+    else if ((TREE_CODE (ops[i]) == VAR_DECL) && !DECL_ARTIFICIAL (ops[i])) {
+      return true;

what about PARM_DECLs?

you use the mf_ssa_eligible_p function only once and the original
implementation didn't seem to care of your constraint - what is the
difference now?  Note that for example the expressiosn for *(p + i + j) could
look like

  D.123 = i * 4;
  D.124 = j * 4;
  D.125 = p + D.123;
  D.126 = D.125 + D.124;
  ... = *D.126;

thus the definition of D.126 wouldn't contain a non-artificial name.
Would it be ok to not instrument this dereference?  Thus I think in
practice you would need to loop a bit here, which then might
justify a pre-walk over the IL collecting eligible names instead
of re-computing them at each dereference.

Thanks,
Richard.


> Thanks
>
>
> tunji
>