On Fri, Mar 04, 2016 at 12:10:26AM -0700, Jeff Law wrote:
> On 03/03/2016 07:35 AM, Jakub Jelinek wrote:
> >Hi!
> >
> >I'd like to ping fix for P1 PR69947:
> >https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01743.html
> So essentially this is just marking more things so that we don't prune them
> away, right?
> 
> It's similar conceptually to one of Pierre-Marie's patches where he removed
> the switch and recursed anytime the operand's val_class matched
> dw_val_class_die_ref and was !external.  Yours just explicitly adds the new
> DW_OP_ things to the switch and has a slightly looser check (dropping the
> !external part of the check).

The !external part is IMHO not needed, as we only set external for
attributes, not for operands of DWARF expression opcodes.
And, while checking both operands for dw_val_class_die_ref is possible, it
is not enough, it doesn't cover the DW_OP_GNU_entry_value case where it is
a val_loc, and that case really depends on the context, other val_locs
shouldn't be traversed.  So, I think it is better to list the opcodes
explicitly, as that gives the needed context to what the arguments are,
perhaps at some point we'll refer to DIEs that we want to do something
different for.
When adding new DW_OP_* values, one has to change dozens of other places
anyway, so one further one doesn't matter, one has to search for all the
spots anyway.

        Jakub

Reply via email to