> > We have wrong code with LTO, too.
>
> I know.
>
> > The problem is that IPA passes (and
> > not only that, loop analysis too) does analysis at compile time (with
> > value numbers in) and streams the info separately.
>
> And that is desirable, because otherwise it simply couldn't derive any
> ranges.
>
> > Removal of value ranges
> > (either by LTO or by your patch) happens between computing these
> > summaries and using them, so this can be used to trigger wrong code,
> > sadly.
>
> Yes. But with LTO, I don't see how the IPA ICF comparison whether
> two functions are the same or not could be done with the
> SSA_NAME_{RANGE,PTR}_INFO in, otherwise it could only ICF merge functions
> from the same TUs. So the comparison IMHO (and the assert checks in my
> patch prove that) is done when the SSA_NAME_{RANGE,PTR}_INFO aren't in
> anymore. So, one just needs to compare and punt or union whatever
> is or could be influenced in the IPA streamed data from the ranges etc.
> And because one has to do it for LTO, doing it for non-LTO should be
> sufficient too.
Sorry, this was bit of a misunderstanding: I tought you still considered
the original patch to be full fix, while I tought I should look into it
more and dig out more issues. This is bit of can of worms. Overall I
think the plan is:
This stage4
1) fix VR divergences by either comparing or droping them
2) fix jump function differences by comparing them
(I just constructed a testcase showing that jump functions can
diverge for other reasons than just VR, so this is deeper problem,
too)
3) try to construct aditional wrong code testcases (finite_p
divergences, trapping)
Next stage1
4) implement VR and PTR info streaming
5) make ICF to compare VRs and punt otherwise
6) implement optimize_size feature to ICF that will not punt on
diverging TBAA or value ranges and do merging instead.
We need some extra infrastructure for that, being able to keep the
maps between basic blocks and SSA names from comparsion stage to
merging stage
7) measure how effective ICF becomes in optimize_size mode and implement
heuristics on how much metadata merging we want to do with -O2/-O3 as
well.
Based on older data Martin collected for his thesis, ICF used to be
must more effective on libreoffice then it is today, so hopefully we
can recover 10-15% binary size improvmeents here.
8) General ICF TLC. There are many half finished things for a while in
this pass (such as merging with different BB or stmt orders).
I am attaching the compare patch which also fixes the original wrong
code. If you preffer your version, just go ahead to commit it. Otherwise
I will add your testcase for this patch and commit this one.
Statistically we almost never merge functions with different value
ranges (three in testsuite, 0 during bootstrap, 1 during LTO bootstrap
and probably few in LLVM build - there are 15 cases reported, but some
are false positives caused by hash function conflicts).
Honza
gcc/ChangeLog:
* ipa-icf-gimple.cc (func_checker::compare_ssa_name): Compare value
ranges.
diff --git a/gcc/ipa-icf-gimple.cc b/gcc/ipa-icf-gimple.cc
index 8c2df7a354e..37c416d777d 100644
--- a/gcc/ipa-icf-gimple.cc
+++ b/gcc/ipa-icf-gimple.cc
@@ -39,9 +39,11 @@ along with GCC; see the file COPYING3. If not see
#include "cfgloop.h"
#include "attribs.h"
#include "gimple-walk.h"
+#include "value-query.h"
+#include "value-range-storage.h"
#include "tree-ssa-alias-compare.h"
#include "ipa-icf-gimple.h"
namespace ipa_icf_gimple {
@@ -109,6 +116,35 @@ func_checker::compare_ssa_name (const_tree t1, const_tree
t2)
else if (m_target_ssa_names[i2] != (int) i1)
return false;
+ if (POINTER_TYPE_P (TREE_TYPE (t1)))
+ {
+ if (SSA_NAME_PTR_INFO (t1))
+ {
+ if (!SSA_NAME_PTR_INFO (t2)
+ || SSA_NAME_PTR_INFO (t1)->align != SSA_NAME_PTR_INFO (t2)->align
+ || SSA_NAME_PTR_INFO (t1)->misalign != SSA_NAME_PTR_INFO
(t2)->misalign)
+ return false;
+ }
+ else if (SSA_NAME_PTR_INFO (t2))
+ return false;
+ }
+ else
+ {
+ if (SSA_NAME_RANGE_INFO (t1))
+ {
+ if (!SSA_NAME_RANGE_INFO (t2))
+ return false;
+ Value_Range r1 (TREE_TYPE (t1));
+ Value_Range r2 (TREE_TYPE (t2));
+ SSA_NAME_RANGE_INFO (t1)->get_vrange (r1, TREE_TYPE (t1));
+ SSA_NAME_RANGE_INFO (t2)->get_vrange (r2, TREE_TYPE (t2));
+ if (r1 != r2)
+ return false;
+ }
+ else if (SSA_NAME_RANGE_INFO (t2))
+ return false;
+ }
+
if (SSA_NAME_IS_DEFAULT_DEF (t1))
{
tree b1 = SSA_NAME_VAR (t1);
