https://github.com/steakhal created
https://github.com/llvm/llvm-project/pull/71284
The idea is that if we see a `X RELOP Y` being constrained to a RangeSet `S`,
then check the eqclasses of X and Y respectively and for `X' RELOP Y'`
SymSymExprs and try to infer their ranges.
If there is no con
llvmbot wrote:
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Balazs Benics (steakhal)
Changes
The idea is that if we see a `X RELOP Y` being constrained to a RangeSet `S`,
then check the eqclasses of X and Y respectively and for `X' RELOP Y'`
SymSymExprs and try to infer their ran
steakhal wrote:
For crossreference: I raised some related questions around having void casts
artificially keeping constraints and symbols alive at discuss:
https://discourse.llvm.org/t/range-based-solver-and-eager-symbol-garbage-collection/74670
https://github.com/llvm/llvm-project/pull/71284
_
Xazax-hun wrote:
I think every time we need to iterate over all member of an equivalence class,
we might do something wrong. The point of the equivalence class would be to
make sure those elements are equivalent. One way to avoid iteration would be to
always use the representative of the equiv
steakhal wrote:
> I think every time we need to iterate over all member of an equivalence
> class, we might do something wrong. The point of the equivalence class would
> be to make sure those elements are equivalent. One way to avoid iteration
> would be to always use the representative of th
https://github.com/DonatNagyE commented:
This is a good and important improvement of the analysis results, so I support
merging it (with some very minor changes), but I feel that it's a "practical,
but incomplete" band-aid instead of a systemic improvement that fits into the
architecture.
The
https://github.com/DonatNagyE edited
https://github.com/llvm/llvm-project/pull/71284
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -2218,11 +2224,58 @@ bool ConstraintAssignor::assignSymExprToConst(const
SymExpr *Sym,
return true;
}
+bool ConstraintAssignor::handleEquivalentAlternativeSymOperands(
+const SymSymExpr *SymSym, RangeSet Constraint) {
+ SymbolRef LHS = SymSym->getLHS();
+ SymbolRef
steakhal wrote:
> @DonatNagyE The most straightforward issue that I see is that (if I
> understand the code correctly) the intersected constraint (the value of the
> variable `Constraint` at the end of
> `handleEquivalentAlternativeSymOperands()`) is just discarded after checking
> that it's
https://github.com/steakhal updated
https://github.com/llvm/llvm-project/pull/71284
>From 92ece501b340c3a2a52b5a4614ddb70bb3e35c93 Mon Sep 17 00:00:00 2001
From: Balazs Benics
Date: Sat, 4 Nov 2023 13:44:28 +0100
Subject: [PATCH 1/3] [analyzer][solver] On SymSym RelOps, check EQClass
members f
DonatNagyE wrote:
> I think we haven't discussed yet the approach of applying the constraint to
> every eqclass member. That would feel like defeating the purpose of eqclasses
> at all.
I only mentioned it because from a high-level perspective it's equivalent to
applying the constraint on the
Xazax-hun wrote:
> So, if I understand you correctly, at the 3rd if statement, we should
> canonicalize the symbol we are constraining by walking every sub-symbol and
> substituting it with its equivalent counterpart (if any), by basically with
> its eqclass' representative symbol.
I am not 1
steakhal wrote:
> > So, if I understand you correctly, at the 3rd if statement, we should
> > canonicalize the symbol we are constraining by walking every sub-symbol and
> > substituting it with its equivalent counterpart (if any), by basically with
> > its eqclass' representative symbol.
>
>
Xazax-hun wrote:
> So to be explicit, no other component would know what canonicalization
> happens within the range-based solver.
Oh, I see! Thanks for the clarification, this sounds good to me.
https://github.com/llvm/llvm-project/pull/71284
___
cf
martong wrote:
> it'd be good to "strengthen" the equality classes and ensure that we assign
> the same RangeSet to each member of an EQClass (or, in other words, we assign
> the RangeSet to an EQClass instead of an individual symbol).
RangeSets (i.e. constraints) are assigned to EQClasses, ch
steakhal wrote:
Any objections for landing this PR? Or should we postpone this for llvm-19?
https://github.com/llvm/llvm-project/pull/71284
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
DonatNagyE wrote:
Hmm, I would prefer a cleaner, more "theoretical" improvement of the
equivalence class handling instead of this "add yet another patch that covers
many, but not all cases" approach.
I'm not opposed to landing this PR in llvm-18 (we have it, and it improves the
user experienc
steakhal wrote:
> I'm not opposed to landing this PR in llvm-18 (we have it, and it improves
> the user experience), but I wouldn't like to have it in llvm-19 and later.
To clarify my intentions, I wanted to land this as it resolves user complaints
(as fixing an open issue), at a likely reason
DonatNagyE wrote:
I'd like to abstain from deciding this question.
Personally I don't like the idea that we add yet another hack that'll remain in
the codebase forever and slows down all other development efforts in this area
(as contributors who want to understand this logic will need to stud
19 matches
Mail list logo