two questions about ConditionalEliminationPhase

[Lukas Stadler]

Hey Miguel,

you're absolutely right, this is a bug. The information added to the state needs to specify that the value is non-null (false).

Your suggestion about the LogicCon/DisjunctionNodes makes sense - and it's not something that is handled by the current system.
(The conditions are picked up by conditional elimination only in later stages, when the node is lowered into control flow - which is too late)

However, we recently had lots of discussions about this phase, and it is scheduled for a major refactoring.
The problem is that this nullness state does not, at the moment, tell you where the information is coming from.
Without this it is hard/impossible to handle all cases around guards correctly (which is why the nullness is not used for guards any more…).

Thanks for the suggestion! I'll add it to the list of things to do while refactoring.

cheers,
 Lukas

On Jul 14, 2013, at 10:59 PM, Garcia Gutierrez Miguel Alfredo <miguelalfredo.garcia at epfl.ch> wrote:

> 
> After staring for what seemed like an eternity to ConditionalEliminationPhase, it slowly began to make sense. But not quite, as the questions below show.
> 
> 
> /--------------------------\
> |Question 1: Is this a bug?|
> \--------------------------/
> 
> One of the cases that
> 
>  private void registerCondition(boolean isTrue, LogicNode condition, ValueNode anchor) {
> 
> handles is
> 
>  } else if (condition instanceof ObjectEqualsNode) {
> 
> where the consequences of the object-equality check are propagated.
> 
> In case of inequality, and one of the comparands known to be null, the code below gains information about the other comparand (ie, must be non-null). That's the idea, right?
> 
>                    if (state.isNull(x) && !state.isNonNull(y)) {
>                        metricObjectEqualsRegistered.increment();
> -------------->         state.addNullness(true, y);
>                    } else if (!state.isNonNull(x) && state.isNull(y)) {
>                        metricObjectEqualsRegistered.increment();
> -------------->         state.addNullness(true, x);
>                    }
> 
> However instead of recording non-nullness, the lines with an arrow above mark nullness. Looks to me it should read: `state.addNullness(false, ` etc. Comments are welcome to understand what's going on.
> 
> 
> 
> /----------------------------\
> |Question 2: Could this work?|
> \----------------------------/
> 
> I haven't looked into all phases, thus the suggestion below could be covered already. Let's see.
> 
> In:
> 
>  private void registerCondition(boolean isTrue, LogicNode condition, ValueNode anchor) {
> 
> three shapes of the condition are of interest (InstanceOfNode, IsNullNode, ObjectEqualsNode) because they allow trickling information from the whole condition down to its components.
> 
> Question: could LogicBinaryNode also be handled? For example,
> 
>  (a) a LogicConjunctionNode known to be true  allows establishing all its components to be true;
>  (b) a LogicDisjunctionNode known to be false allows establishing all its components to be false;
> 
> (Not sure whether the extra precision pays off, my question is rather about whether that could be done in principle).
> 
> 
> 
> And now the kudos: Cool stuff!
> 
> 
> Miguel
> 
> 
> -- 
> Miguel Garcia
> Swiss Federal Institute of Technology
> EPFL - IC - LAMP1 - INR 328 - Station 14
> CH-1015 Lausanne - Switzerland
> http://lamp.epfl.ch/~magarcia/
>