Skip to content

Translate _d_arraycatnTX to a template #14550

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

Merged
RazvanN7 merged 1 commit into from
Apr 28, 2023
Merged

Translate _d_arraycatnTX to a template #14550

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
61 changes: 40 additions & 21 deletions compiler/src/dmd/dinterpret.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5750,7 +5750,25 @@ public:
e2 = ue2.copy();
}

*pue = ctfeCat(e.loc, e.type, e1, e2);
Expression prepareCatOperand(Expression exp)
{
/* Convert `elem ~ array` to `[elem] ~ array` if `elem` is itself an
* array. This is needed because interpreting the `CatExp` calls
* `Cat()`, which cannot handle concatenations between different
* types, except for strings and chars.
*/
auto tb = e.type.toBasetype();
auto tbNext = tb.nextOf();
auto expTb = exp.type.toBasetype();

if (exp.type.implicitConvTo(tbNext) >= MATCH.convert &&
(tb.ty == Tarray || tb.ty == Tsarray) &&
(expTb.ty == Tarray || expTb.ty == Tsarray))
return new ArrayLiteralExp(exp.loc, e.type, exp);
return exp;
}

*pue = ctfeCat(e.loc, e.type, prepareCatOperand(e1), prepareCatOperand(e2));
result = pue.exp();

if (CTFEExp.isCantExp(result))
Expand Down Expand Up @@ -6442,33 +6460,34 @@ void interpretThrow(ref Expression result, Expression exp, const ref Loc loc, In
}
}

/*********************************************
* Checks if the given expresion is a call to the runtime hook `id`.
* Params:
* e = the expression to check
* id = the identifier of the runtime hook
* Returns:
* `e` cast to `CallExp` if it's the hook, `null` otherwise
*/
private CallExp isRuntimeHook(Expression e, Identifier id)
/*********************************************
* Checks if the given expresion is a call to the runtime hook `id`.
*
* Params:
* e = the expression to check
* id = the identifier of the runtime hook
* Returns:
* `e` cast to `CallExp` if it's the hook, `null` otherwise
*/
public CallExp isRuntimeHook(Expression e, Identifier id)
{
if (auto ce = e.isCallExp())
{
if (auto ce = e.isCallExp())
if (auto ve = ce.e1.isVarExp())
{
if (auto ve = ce.e1.isVarExp())
if (auto fd = ve.var.isFuncDeclaration())
{
if (auto fd = ve.var.isFuncDeclaration())
{
// If `_d_HookTraceImpl` is found, resolve the underlying
// hook and replace `e` and `fd` with it.
removeHookTraceImpl(ce, fd);
return fd.ident == id ? ce : null;
}
// If `_d_HookTraceImpl` is found, resolve the underlying hook
// and replace `e` and `fd` with it.
removeHookTraceImpl(ce, fd);
return fd.ident == id ? ce : null;
}
}

return null;
}

return null;
}

/********************************************
* Interpret the expression.
* Params:
Expand Down
46 changes: 3 additions & 43 deletions compiler/src/dmd/e2ir.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1725,50 +1725,10 @@ elem* toElem(Expression e, IRState *irs)
return el_long(TYint, 0);
}

Copy link
Copy Markdown
Contributor

@RazvanN7 RazvanN7 Apr 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above betterC check could be probably moved to the frontend. You could check sc.func.needsCodegen, however, that is a topic for a different PR.

Type tb1 = ce.e1.type.toBasetype();
Type tb2 = ce.e2.type.toBasetype();

Type ta = (tb1.ty == Tarray || tb1.ty == Tsarray) ? tb1 : tb2;

elem *e;
if (ce.e1.op == EXP.concatenate)
{
CatExp ex = ce;

// Flatten ((a ~ b) ~ c) to [a, b, c]
Elems elems;
elems.shift(array_toDarray(ex.e2.type, toElem(ex.e2, irs)));
do
{
ex = cast(CatExp)ex.e1;
elems.shift(array_toDarray(ex.e2.type, toElem(ex.e2, irs)));
} while (ex.e1.op == EXP.concatenate);
elems.shift(array_toDarray(ex.e1.type, toElem(ex.e1, irs)));
if (auto lowering = ce.lowering)
return toElem(lowering, irs);

// We can't use ExpressionsToStaticArray because each exp needs
// to have array_toDarray called on it first, as some might be
// single elements instead of arrays.
Symbol *sdata;
elem *earr = ElemsToStaticArray(ce.loc, ce.type, &elems, &sdata);

elem *ep = el_pair(TYdarray, el_long(TYsize_t, elems.length), el_ptr(sdata));
if (irs.target.os == Target.OS.Windows && irs.target.is64bit)
ep = addressElem(ep, Type.tvoid.arrayOf());
ep = el_param(ep, getTypeInfo(ce, ta, irs));
e = el_bin(OPcall, TYdarray, el_var(getRtlsym(RTLSYM.ARRAYCATNTX)), ep);
toTraceGC(irs, e, ce.loc);
e = el_combine(earr, e);
}
else
{
elem *e1 = eval_Darray(ce.e1);
elem *e2 = eval_Darray(ce.e2);
elem *ep = el_params(e2, e1, getTypeInfo(ce, ta, irs), null);
e = el_bin(OPcall, TYdarray, el_var(getRtlsym(RTLSYM.ARRAYCATT)), ep);
toTraceGC(irs, e, ce.loc);
}
elem_setLoc(e,ce.loc);
return e;
assert(0, "This case should have been rewritten to `_d_arraycatnTX` in the semantic phase");
}

/***************************************
Expand Down
4 changes: 3 additions & 1 deletion compiler/src/dmd/expression.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1542,7 +1542,7 @@ extern (C++) abstract class Expression : ASTNode
// so don't print anything to avoid double error messages.
if (!(f.ident == Id._d_HookTraceImpl || f.ident == Id._d_arraysetlengthT
|| f.ident == Id._d_arrayappendT || f.ident == Id._d_arrayappendcTX
|| f.ident == Id._d_newclassT))
|| f.ident == Id._d_arraycatnTX || f.ident == Id._d_newclassT))
{
error("`@nogc` %s `%s` cannot call non-@nogc %s `%s`",
sc.func.kind(), sc.func.toPrettyChars(), f.kind(), f.toPrettyChars());
Expand Down Expand Up @@ -6555,6 +6555,8 @@ extern (C++) final class MinExp : BinExp
*/
extern (C++) final class CatExp : BinExp
{
Expression lowering; // call to druntime hook `_d_arraycatnTX`

extern (D) this(const ref Loc loc, Expression e1, Expression e2) scope
{
super(loc, EXP.concatenate, e1, e2);
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dmd/expression.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1207,6 +1207,8 @@ class MinExp final : public BinExp
class CatExp final : public BinExp
{
public:
Expression *lowering; // call to druntime hook `_d_arraycatnTX`

void accept(Visitor *v) override { v->visit(this); }
};

Expand Down
105 changes: 92 additions & 13 deletions compiler/src/dmd/expressionsem.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2176,7 +2176,11 @@ private bool functionParameters(const ref Loc loc, Scope* sc,
}
// Allow 'lazy' to imply 'scope' - lazy parameters can be passed along
// as lazy parameters to the next function, but that isn't escaping.
else if (!(pStc & STC.lazy_))
// The arguments of `_d_arraycatnTX` are already handled in
// expressionsem.d, via `checkNewEscape`. Without `-dip1000`, the
// check does not return an error, so the lowering of `a ~ b` to
// `_d_arraycatnTX(a, b)` still occurs.
else if (!(pStc & STC.lazy_) && (!fd || fd.ident != Id._d_arraycatnTX))
{
/* Argument value can escape from the called function.
* Check arg to see if it matters.
Expand Down Expand Up @@ -2207,6 +2211,7 @@ private bool functionParameters(const ref Loc loc, Scope* sc,
// allocate the array literal as temporary static array on the stack
ale.type = ale.type.nextOf().sarrayOf(ale.elements.length);
auto tmp = copyToTemp(0, "__arrayliteral_on_stack", ale);
tmp.storage_class |= STC.exptemp;
auto declareTmp = new DeclarationExp(ale.loc, tmp);
auto castToSlice = new CastExp(ale.loc, new VarExp(ale.loc, tmp),
p.type.substWildTo(MODFlags.mutable));
Expand Down Expand Up @@ -10930,6 +10935,86 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
return;
}

/**
* If the given expression is a `CatExp`, the function tries to lower it to
* `_d_arraycatnTX`.
*
* Params:
* ee = the `CatExp` to lower
* Returns:
* `_d_arraycatnTX(e1, e2, ..., en)` if `ee` is `e1 ~ e2 ~ ... en`
* `ee` otherwise
*/
private Expression lowerToArrayCat(CatExp exp)
{
// String literals are concatenated by the compiler. No lowering is needed.
if ((exp.e1.isStringExp() && (exp.e2.isIntegerExp() || exp.e2.isStringExp())) ||
(exp.e2.isStringExp() && (exp.e1.isIntegerExp() || exp.e1.isStringExp())))
return exp;

Identifier hook = global.params.tracegc ? Id._d_arraycatnTXTrace : Id._d_arraycatnTX;
if (!verifyHookExist(exp.loc, *sc, hook, "concatenating arrays"))
{
setError();
return result;
}

void handleCatArgument(Expressions *arguments, Expression e)
{
if (auto ce = e.isCatExp())
{
Expression lowering = ce.lowering;

/* Skip `file`, `line`, and `funcname` if the hook of the parent
* `CatExp` is `_d_arraycatnTXTrace`.
*/
if (auto callExp = isRuntimeHook(lowering, hook))
{
if (hook == Id._d_arraycatnTX)
arguments.pushSlice((*callExp.arguments)[]);
else
arguments.pushSlice((*callExp.arguments)[3 .. $]);
}
}
else
arguments.push(e);
}

auto arguments = new Expressions();
if (global.params.tracegc)
{
auto funcname = (sc.callsc && sc.callsc.func) ?
sc.callsc.func.toPrettyChars() : sc.func.toPrettyChars();
arguments.push(new StringExp(exp.loc, exp.loc.filename.toDString()));
arguments.push(new IntegerExp(exp.loc, exp.loc.linnum, Type.tint32));
arguments.push(new StringExp(exp.loc, funcname.toDString()));
}

handleCatArgument(arguments, exp.e1);
handleCatArgument(arguments, exp.e2);

Expression id = new IdentifierExp(exp.loc, Id.empty);
id = new DotIdExp(exp.loc, id, Id.object);

auto tiargs = new Objects();
tiargs.push(exp.type);
id = new DotTemplateInstanceExp(exp.loc, id, hook, tiargs);
id = new CallExp(exp.loc, id, arguments);
return id.expressionSemantic(sc);
}

void trySetCatExpLowering(Expression exp)
{
/* `_d_arraycatnTX` canot be used with `-betterC`, but `CatExp`s may be
* used with `-betterC`, but only during CTFE.
*/
if (global.params.betterC)
return;

if (auto ce = exp.isCatExp())
ce.lowering = lowerToArrayCat(ce);
}

override void visit(CatExp exp)
{
// https://dlang.org/spec/expression.html#cat_expressions
Expand Down Expand Up @@ -11017,14 +11102,10 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
exp.e2 = exp.e2.implicitCastTo(sc, tb1next);
exp.type = tb1next.arrayOf();
L2elem:
if (tb2.ty == Tarray || tb2.ty == Tsarray)
{
// Make e2 into [e2]
exp.e2 = new ArrayLiteralExp(exp.e2.loc, exp.type, exp.e2);
}
else if (checkNewEscape(sc, exp.e2, false))
if (checkNewEscape(sc, exp.e2, false))
return setError();
result = exp.optimize(WANTvalue);
trySetCatExpLowering(result);
return;
}
}
Expand Down Expand Up @@ -11055,14 +11136,10 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
exp.e1 = exp.e1.implicitCastTo(sc, tb2next);
exp.type = tb2next.arrayOf();
L1elem:
if (tb1.ty == Tarray || tb1.ty == Tsarray)
{
// Make e1 into [e1]
exp.e1 = new ArrayLiteralExp(exp.e1.loc, exp.type, exp.e1);
}
else if (checkNewEscape(sc, exp.e1, false))
if (checkNewEscape(sc, exp.e1, false))
return setError();
result = exp.optimize(WANTvalue);
trySetCatExpLowering(result);
return;
}
}
Expand All @@ -11085,6 +11162,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
if (Expression ex = typeCombine(exp, sc))
{
result = ex;
trySetCatExpLowering(result);
return;
}
exp.type = exp.type.toHeadMutable();
Expand Down Expand Up @@ -11117,6 +11195,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
}

result = e;
trySetCatExpLowering(result);
}

override void visit(MulExp exp)
Expand Down
3 changes: 3 additions & 0 deletions compiler/src/dmd/frontend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7872,6 +7872,7 @@ class MinExp final : public BinExp
class CatExp final : public BinExp
{
public:
Expression lowering;
Expression* resolveLoc(const Loc& loc, Scope* sc) override;
void accept(Visitor* v) override;
};
Expand Down Expand Up @@ -8731,6 +8732,8 @@ struct Id final
static Identifier* _d_arrayappendcTXImpl;
static Identifier* _d_arrayappendcTX;
static Identifier* _d_arrayappendcTXTrace;
static Identifier* _d_arraycatnTX;
static Identifier* _d_arraycatnTXTrace;
static Identifier* stdc;
static Identifier* stdarg;
static Identifier* va_start;
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dmd/id.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,8 @@ immutable Msgtable[] msgtable =
{ "_d_arrayappendcTXImpl" },
{ "_d_arrayappendcTX" },
{ "_d_arrayappendcTXTrace" },
{ "_d_arraycatnTX" },
{ "_d_arraycatnTXTrace" },

// varargs implementation
{ "stdc" },
Expand Down
27 changes: 27 additions & 0 deletions compiler/src/dmd/inline.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,21 @@ public:
result = ae;
}

override void visit(CatExp e)
{
auto ce = e.copy().isCatExp();

if (auto lowering = ce.lowering)
ce.lowering = doInlineAs!Expression(lowering, ids);
else
{
ce.e1 = doInlineAs!Expression(e.e1, ids);
ce.e2 = doInlineAs!Expression(e.e2, ids);
}

result = ce;
}

override void visit(BinExp e)
{
auto be = cast(BinExp)e.copy();
Expand Down Expand Up @@ -1241,6 +1256,18 @@ public:
inlineScan(e.msg);
}

override void visit(CatExp e)
{
if (auto lowering = e.lowering)
{
inlineScan(lowering);
return;
}

inlineScan(e.e1);
inlineScan(e.e2);
}

override void visit(BinExp e)
{
inlineScan(e.e1);
Expand Down
0 compiler/test/runnable/test19688.d → compiler/test/compilable/test19688.d
View file
Open in desktop
Comment thread
JohanEngelen marked this conversation as resolved.
File renamed without changes.
4 changes: 3 additions & 1 deletion compiler/test/runnable/extra-files/hello-profile-postscript.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
source tools/common_funcs.sh

# strip out Dmain since it's symbol differs between windows and non-windows
grep -v Dmain ${OUTPUT_BASE}.d.trace.def > ${OUTPUT_BASE}.d.trace.def2
# strip out _d_arraycatnTX and _d_arraysetlengthT since they are part of the
# lowering of the array concatenation operator
grep -v 'Dmain\|_d_arraycatnTX\|_d_arraysetlengthT' ${OUTPUT_BASE}.d.trace.def > ${OUTPUT_BASE}.d.trace.def2

diff -up --strip-trailing-cr ${EXTRA_FILES}/${TEST_NAME}.d.trace.def ${OUTPUT_BASE}.d.trace.def2

Expand Down
Loading