Hi, On Sun, Oct 11 2020, Jan Hubicka wrote: > Hi, > this patch fixes the range tracking in argument and re-enables it for clones > (the bug that broke dealII and x264 benchmarks) > > It turned out that there was three problems > 1) for SRA/ipa-cp clones we did not update summarries to represent new > signature. This is now done in modref_transform. > I tested it in ipa-sra testcases and it seems to work fine, but Martin, > can you please take a look? > > Param adjustment interface provides original indexes for new indexes of > parameters. I need reverse information: for original index I need new > that I compute via array map and then do the rewrite.
Looks OK, I only wonder whether the functionality cannot be more generally useful and so whether computation of the reverse map should not be made part of ipa_param_adjustments interface. > > Martin, if things passed by references are translated to stuff > passed by value, we may eliminate the corrsponding acess records, > because reading function parameters is not considered a side-effect > by mod-ref. Is there easy way to detect such change? Such new parameter will have IPA_PARAM_OP_SPLIT as the operation in its ipa_adjusted_param and so ipa_param_adjustments::get_original_index will already return -1 for it, if that is enough. The way to detect that is currently only the fact that a pointer argument is split (new IPA SRA no longer splits a reference into sub-references). If the old parameter is no longer available we need to add a flag, I'm afraid. > > 2) The propagation via jump functions (in applying inline decision) > mixed bits and bytes in parameter offsets. > Martin, it is not clear to me why the offset is in bits - there is no > way to pass a pointer to non-byte aligned address and it seems that this > only adds a risk of overflows. Yeah, when I started writing the first versions of the code I simply used directly the types that get_ref_base_and_extent uses. I hope it's all in HOST_WIDE_INTs and so should not really overflow but it wastes memory. Converting all of this to bytes is on my TODO list (and new IPA-SRA already does that in the IPA phase). > There seems to be overflow check missing in > update_jump_functions_after_inlining, but it seems to me that these days > this should be poly_int64. Not sure about this. When the conversion to poly_ints happened, get_ref_base_and_extent_hwi was introduced for the purposes of IPA stuff, so I think even the poly_int authors did think it was the right thing to use here. In any case, I'm still having hard time wrapping my head around poly_ints, so I'd like to see a testcase before going down this route. Thanks, Martin
