Skip to content

Commit

Permalink
fuir: do not create new clazzes when lookupDone==true
Browse files Browse the repository at this point in the history
fixes #4237
  • Loading branch information
michaellilltokiwa committed Jan 21, 2025
1 parent 95a902e commit 37bfe12
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 58 deletions.
20 changes: 5 additions & 15 deletions src/dev/flang/fuir/Clazz.java
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ enum LayoutStatus
/**
* Integer id of this Clazz used in FUIR instance.
*/
final int _id;
int _id = -1;


/**
Expand Down Expand Up @@ -282,14 +282,11 @@ enum LayoutStatus
*
* @param select in case actualType refers to a field whose result type is an
* open generic parameter, select specifies the actual generic to be used.
*
* @param id the inter id used by FUIR to identify this Clazz.
*/
Clazz(GeneratingFUIR fuir,
Clazz outer,
AbstractType type,
int select,
int id)
int select)
{
if (PRECONDITIONS) require
(!type.dependsOnGenerics() || true /* NYI: UNDER DEVELOPMENT: Why? */,
Expand All @@ -307,7 +304,6 @@ enum LayoutStatus

_outer = outer;
_select = select;
_id = id;
_needsCode = false;
_code = IR.NO_SITE;
}
Expand All @@ -318,8 +314,9 @@ enum LayoutStatus
* Additional initialization code that has to be run after this Clazz was
* added to FUIRI._clazzes for recursive clazz lookup.
*/
void init()
void init(int id)
{
_id = id;
_choiceGenerics = determineChoiceGenerics();
var vas = feature().valueArguments();
if (vas.size() == 0 || isBoxed())
Expand Down Expand Up @@ -365,16 +362,9 @@ void init()
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
68 changes: 26 additions & 42 deletions src/dev/flang/fuir/GeneratingFUIR.java
Original file line number Diff line number Diff line change
Expand Up @@ -331,25 +331,24 @@ 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, CLAZZ_BASE + _clazzes.size());
var cl = new Clazz(this, outerR, t, select);
var existing = _clazzesTM.get(cl);
if (existing != null)
{
result = existing;
}
else
{
if (CHECKS) check
(!_lookupDone);

result = cl;
var fuirId = CLAZZ_BASE + _clazzes.size();
_clazzes.add(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 (CHECKS) check
(_clazzes.get(clazzId2num(fuirId)) == cl);

if (CACHE_RESULT_CLAZZ && _clazzes.size() > _resultClazzes.length)
{
var rc = _resultClazzes;
Expand Down Expand Up @@ -407,11 +406,11 @@ Clazz newClazz(Clazz outerR, AbstractType actualType, int select)
}
cl._specialClazzId = s;
if (SHOW_NEW_CLAZZES) System.out.println("NEW CLAZZ "+cl);
cl.init();
cl.init(fuirId);

result.registerAsHeir();

// C backend requires the value variant for all ref clazzes, so we make
// backends require the value variant for all ref clazzes, so we make
// sure we have the value clazz as well:
var ignore = clazzAsValue(result._id);
}
Expand Down Expand Up @@ -1383,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 @@ -1855,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 @@ -1866,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 @@ -1958,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 @@ -1969,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 @@ -2058,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 @@ -2070,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 @@ -2300,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 @@ -2397,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 @@ -2407,6 +2393,8 @@ public void lookupDone()
var c = _clazzes.get(i);
c.layoutAndHandleCycle();
}

_lookupDone = true;
}


Expand Down Expand Up @@ -2499,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 @@ -2545,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 @@ -2556,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
3 changes: 2 additions & 1 deletion src/dev/flang/fuir/analysis/TailCall.java
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,8 @@ private boolean isTailCall(int cl, int cls, int s, int mustAssignTo)
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 &&
(cc == IR.NO_CLAZZ || 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

0 comments on commit 37bfe12

Please sign in to comment.