Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fuir: do not create new clazzes when lookupDone==true #4643

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+38 −56
Open

fuir: do not create new clazzes when lookupDone==true #4643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
82396d5
fuir: do not create new clazzes when lookupDone==true
michaellilltokiwa Jan 22, 2025
2f9fbed
Merge branch 'main' into issue/4237_1
michaellilltokiwa Jan 22, 2025
f686972
Merge branch 'main' into issue/4237_1
michaellilltokiwa Jan 22, 2025
59ffa39
be/int: do not set fields where resultClazz is NO_CLAZZ
michaellilltokiwa Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/dev/flang/be/interpreter/Interpreter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,14 @@ static void setField(int thiz, int staticClazz, Value curValue, Value v)
);

int fclazz = clazzForField(thiz);
LValue slot = fieldSlot(thiz, staticClazz, fclazz, curValue);
setFieldSlot(thiz, fclazz, slot, v);
// if fclazz == FUIR.NO_CLAZZ
// this likely means field was never read
// during DFA phase.
if (fclazz != FUIR.NO_CLAZZ)
{
LValue slot = fieldSlot(thiz, staticClazz, fclazz, curValue);
setFieldSlot(thiz, fclazz, slot, v);
}
}


Expand Down
9 changes: 1 addition & 8 deletions src/dev/flang/fuir/Clazz.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,16 +362,9 @@ void init(int id)
void addInner(Clazz i)
{
if (PRECONDITIONS) require
(true || !_fuir._lookupDone /* NYI: UNDER DEVELOPMENT: precondition does not hold yet */ ,
(!_fuir._lookupDone,
i.clazzKind() != IR.FeatureKind.Field || isRef() == YesNo.no);

if (_fuir._lookupDone)
{
if (false)
{ // NYI: CLEANUP: this should no longer happen, but it happens during layout phase, need to check why.
throw new Error("ADDING "+i+" to "+this);
}
}
_inner.add(i);
}

Expand Down
57 changes: 18 additions & 39 deletions src/dev/flang/fuir/GeneratingFUIR.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,6 @@ Clazz newClazz(Clazz outerR, AbstractType actualType, int select)

// normalize outer to be value in case t describes a field
outerR = t.feature().isField() ? outerR.asValue() : outerR;

var cl = new Clazz(this, outerR, t, select);
var existing = _clazzesTM.get(cl);
if (existing != null)
Expand All @@ -340,21 +339,16 @@ Clazz newClazz(Clazz outerR, AbstractType actualType, int select)
}
else
{
if (CHECKS) check
(!_lookupDone);

result = cl;
var fuirId = CLAZZ_BASE + _clazzes.size();
_clazzes.add(cl);

if (CHECKS) check
(_clazzes.get(clazzId2num(fuirId)) == cl);

if (_lookupDone)
{
if (false) // NYI: BUG: #4273: This still happens for some tests and
// some backends, need to check why and avoid this!
{
throw new Error("FUIR is closed, but we are adding a new clazz " + cl + " #"+clazzId2num(cl._id));
}
}
if (CACHE_RESULT_CLAZZ && _clazzes.size() > _resultClazzes.length)
{
var rc = _resultClazzes;
Expand Down Expand Up @@ -1388,7 +1382,7 @@ Clazz specialClazz(SpecialClazzes s)
(s != SpecialClazzes.c_NOT_FOUND);

var result = _specialClazzes[s.ordinal()];
if (result == null)
if (result == null && !_lookupDone)
{
if (s == SpecialClazzes.c_universe)
{
Expand Down Expand Up @@ -1860,9 +1854,7 @@ public ExprKind codeAt(int s)
*
* @return pair of untagged and tagged types.
*/
private
synchronized /* NYI: remove once it is ensured that _siteClazzCache is no longer modified when _lookupDone */
Pair<Clazz,Clazz> tagValueAndResultClazz(int s)
private Pair<Clazz,Clazz> tagValueAndResultClazz(int s)
{
if (PRECONDITIONS) require
(s >= SITE_BASE,
Expand All @@ -1871,10 +1863,8 @@ Pair<Clazz,Clazz> tagValueAndResultClazz(int s)
codeAt(s) == ExprKind.Tag);

var res = (Pair<Clazz,Clazz>) _siteClazzCache.get(s);
if (res == null)
if (res == null && !_lookupDone)
{
if (CHECKS) check
(!_lookupDone);
var cl = clazzAt(s);
var outerClazz = clazz(cl);
var t = (Tag) getExpr(s);
Expand Down Expand Up @@ -1963,9 +1953,7 @@ public int tagTagNum(int s)
*
* @return a pair consisting of the original type and the new boxed type
*/
private
synchronized /* NYI: remove once it is ensured that _siteClazzCache is no longer modified when _lookupDone */
Pair<Clazz,Clazz> boxValueAndResultClazz(int s)
private Pair<Clazz,Clazz> boxValueAndResultClazz(int s)
{
if (PRECONDITIONS) require
(s >= SITE_BASE,
Expand All @@ -1974,10 +1962,8 @@ Pair<Clazz,Clazz> boxValueAndResultClazz(int s)
codeAt(s) == ExprKind.Box);

var res = (Pair<Clazz,Clazz>) _siteClazzCache.get(s);
if (res == null)
if (res == null && !_lookupDone)
{
if (CHECKS) check
(!_lookupDone || true); // NYI, why doesn't this hold?
var cl = clazzAt(s);
var outerClazz = id2clazz(cl);
var b = (Box) getExpr(s);
Expand Down Expand Up @@ -2063,9 +2049,7 @@ public String comment(int s)
* assignment to a field that is unused, so the assignment is not needed.
*/
@Override
public
synchronized /* NYI: remove once it is ensured that _siteClazzCache is no longer modified when _lookupDone */
int accessedClazz(int s)
public int accessedClazz(int s)
{
if (PRECONDITIONS) require
(s >= SITE_BASE,
Expand All @@ -2075,10 +2059,8 @@ int accessedClazz(int s)
codeAt(s) == ExprKind.Assign );

var res = _siteClazzCache.get(s);
if (res == null)
if (res == null && !_lookupDone)
{
if (CHECKS) check
(!_lookupDone || true); // NYI, why doesn't this hold?
res = accessedClazz(s, null);
if (res == null)
{
Expand Down Expand Up @@ -2305,9 +2287,7 @@ private int[] accessedClazzesDynamic(int s)
* feature to be accessed for this target.
*/
@Override
public
synchronized /* NYI: remove once it is ensured that _siteClazzCache is no longer modified when _lookupDone */
int[] accessedClazzes(int s)
public int[] accessedClazzes(int s)
{
if (PRECONDITIONS) require
(s >= SITE_BASE,
Expand Down Expand Up @@ -2402,7 +2382,8 @@ public void lookupDone()
if (CHECKS) check
(!_lookupDone);

_lookupDone = true;
// NYI: lookupDone before layout
// _lookupDone = true;

// NYI: UNDER DEVELOPMENT: layout phase creates new clazzes, which is why we cannot iterate like this. Need to check why and remove this!
//
Expand All @@ -2412,6 +2393,8 @@ public void lookupDone()
var c = _clazzes.get(i);
c.layoutAndHandleCycle();
}

_lookupDone = true;
}


Expand Down Expand Up @@ -2504,7 +2487,7 @@ public int constClazz(int s)
codeAt(s) == ExprKind.Const);

var res = (Clazz) _siteClazzCache.get(s);
if (res == null)
if (res == null && !_lookupDone)
{
var cl = clazzAt(s);
var cc = id2clazz(cl);
Expand Down Expand Up @@ -2550,9 +2533,7 @@ public byte[] constData(int s)
* @return clazz id of type of the subject
*/
@Override
public
synchronized /* NYI: remove once it is ensured that _siteClazzCache is no longer modified when _lookupDone */
int matchStaticSubject(int s)
public int matchStaticSubject(int s)
{
if (PRECONDITIONS) require
(s >= SITE_BASE,
Expand All @@ -2561,10 +2542,8 @@ int matchStaticSubject(int s)
codeAt(s) == ExprKind.Match);

var rc = (Clazz) _siteClazzCache.get(s);
if (rc == null)
if (rc == null && !_lookupDone)
{
if (CHECKS) check
(!_lookupDone);
var cl = clazzAt(s);
var cc = id2clazz(cl);
var outerClazz = cc;
Expand Down
18 changes: 11 additions & 7 deletions src/dev/flang/fuir/analysis/TailCall.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

package dev.flang.fuir.analysis;

import static dev.flang.ir.IR.NO_CLAZZ;

import dev.flang.fuir.FUIR;

import dev.flang.ir.IR;
Expand Down Expand Up @@ -107,7 +109,8 @@ public boolean callIsTailCall(int cl, int s)

var c2 = _fuir.clazzCode(cl);
return _fuir.codeSize(c2) > 0 &&
isTailCall(cl, _fuir.codeBlockEnd(c2), s, _fuir.clazzResultField(cl));
(_fuir.alwaysResultsInVoid(s) ||
isTailCall(cl, _fuir.codeBlockEnd(c2), s, _fuir.clazzResultField(cl)));
}


Expand Down Expand Up @@ -160,10 +163,10 @@ public boolean firstArgIsOuter(int s)
*/
private int noClazzIfResultUnitType(int f)
{
if (f != IR.NO_CLAZZ &&
if (f != NO_CLAZZ &&
_fuir.clazzIsUnitType(_fuir.clazzResultClazz(f)))
{
f = IR.NO_CLAZZ;
f = NO_CLAZZ;
}
return f;
}
Expand Down Expand Up @@ -207,7 +210,7 @@ private boolean isTailCall(int cl, int cls, int s, int mustAssignTo)
case Call ->
{
var cc = _fuir.accessedClazz(cls);
yield mustAssignTo == IR.NO_CLAZZ &&
yield mustAssignTo == NO_CLAZZ &&
(// we found call c/ix and we do not need to assign any variable, so we have a success!
cls == s ||

Expand All @@ -222,15 +225,16 @@ private boolean isTailCall(int cl, int cls, int s, int mustAssignTo)
case Assign ->
{
var cc = _fuir.accessedClazz(cls);
if (cc != IR.NO_CLAZZ &&
if (cc != NO_CLAZZ &&
_fuir.clazzIsUnitType(_fuir.clazzResultClazz(cc)))
{
cc = IR.NO_CLAZZ;
cc = NO_CLAZZ;
}
yield
// if this is an assignment to 'Current.mustAssignTo' with, recursively check if
// the value assigned is the call s.
sameField(cc, mustAssignTo) && cls > _fuir.codeBlockStart(cls)+1 &&
sameField(cc, mustAssignTo) &&
cls > _fuir.codeBlockStart(cls)+1 &&
_fuir.codeAt(_fuir.codeIndex(cls, -1)) == IR.ExprKind.Current &&
isTailCall(cl, _fuir.codeIndex(cls, -2), s, -1);
}
Expand Down
Loading