newCTFE rebase-dump #14243

maxhaton · 2022-06-23T05:06:32Z

This adds a preview flag but it shouldn't be merged just yet but you can play with it yourself.

I'll add a markdown file that explains the design in more depth at some point.

dlang-bot · 2022-06-23T05:06:35Z

Thanks for your pull request, @maxhaton!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#14243"

thewilsonator

Review round on fpconv_ctfe.d

thewilsonator · 2022-06-25T00:49:37Z

src/dmd/ctfe/fpconv_ctfe.d

+    int exp;
+};
+
+enum seperate_arrays = 1;


Note sure why this is needed. IMO, just pick a value.

thewilsonator · 2022-06-25T00:51:10Z

src/dmd/ctfe/fpconv_ctfe.d

+enum npowers = 87;
+enum steppowers = 8;
+enum firstpower = -348; /* 10 ^ -348 */
+
+enum expmax = -32;
+enum expmin = -60;
+
+enum expbits = 11; // 64 - 1 /*for signbit*/ - fracbits;
+enum fracbits = 52;
+
+enum fracmask = (1UL << fracbits) - 1;
+enum hiddenbit =  (1UL << fracbits);
+enum expmask = ((1UL << expbits) - 1) << fracbits;
+enum signbit = (1UL << (double.sizeof * 8) -1); // 1 << (64 - 1)
+enum expbias = (((1 << (expbits - 1)) -1) + fracbits);


these should be members of Fp

thewilsonator · 2022-06-25T00:52:06Z

src/dmd/ctfe/fpconv_ctfe.d

+    return bits;
+}
+
+Fp build_fp(double d)


this should be a constructor of Fp

thewilsonator · 2022-06-25T00:52:34Z

src/dmd/ctfe/fpconv_ctfe.d

+static assert(build_double(Fp(9007199254740991LU, 971)) == double.max);
+static assert(() { return build_double(build_fp(double.max)); } () == double.max);
+
+void normalize(Fp* fp)


this should be a method

thewilsonator · 2022-06-25T00:53:11Z

src/dmd/ctfe/fpconv_ctfe.d

+    fp.exp -= shift;
+}
+
+void get_normalized_boundaries(Fp* fp, Fp* lower, Fp* upper)


this should be a method and lower and upper should be out

thewilsonator · 2022-06-25T00:53:44Z

src/dmd/ctfe/fpconv_ctfe.d

+    lower.exp = upper.exp;
+}
+
+Fp multiply(const Fp* a, const Fp* b)


this should be opBinary!("*")

thewilsonator · 2022-06-25T00:55:32Z

src/dmd/ctfe/fpconv_ctfe.d

+    }
+}
+
+static int grisu2(double d, char* digits, int* K)


these static functions should be private

thewilsonator · 2022-06-25T00:55:40Z

src/dmd/ctfe/fpconv_ctfe.d

+    return generate_digits(&w, &upper, &lower, digits, K);
+}
+
+static int emit_digits(char* digits, int ndigits, char* dest, int K, bool neg)


thewilsonator

Review round for ctfe/bc.d

thewilsonator · 2022-06-25T00:58:16Z

src/dmd/ctfe/bc.d

+} +/
+
+
+static if (is(typeof({import dmd.globals : Loc; })))


I presume this will always be true for building dmd. Why is this a thing?

thewilsonator · 2022-06-25T00:59:30Z

src/dmd/ctfe/bc.d

+static assert(!bc_order_errors.length, bc_order_errors);
+
+
+pragma(msg, 2 ^^ 7 - LongInst.max, " opcodes remaining");


put this behind a debug static if of some kind, or just make it a static assert

thewilsonator · 2022-06-25T01:00:30Z

src/dmd/ctfe/bc.d

+    Compiled,
+}
+
+//static if (is(typeof(() { import dmd.declaration : FuncDeclaration; })))


thewilsonator · 2022-06-25T01:01:33Z

src/dmd/ctfe/bc.d

+
+struct BCFunction
+{
+    void* funcDecl;


is this always a FuncDeclaration? if it is the type should reflect this.

thewilsonator · 2022-06-25T01:02:43Z

src/dmd/ctfe/bc.d

+        }
+    }
+
+    extern (D) BCValue genLocal(BCType bct, string name)


why is this explicitly marked extern(D)?

because it uses the string type which will die with the default extern(C++) used for visitors.

its not part of a visitor interface though. I don't follow.

thewilsonator · 2022-06-25T01:09:00Z

src/dmd/ctfe/bc.d

+        emitLongInst(LongInst.HeapStore8, _to.stackAddr, value.stackAddr);
+    }
+
+    void Load16(BCValue _to, BCValue from)


these LoadNN and StoreNN look very similar, there should probably be a common definition.

thewilsonator · 2022-06-25T01:09:44Z

src/dmd/ctfe/bc.d

+
+    BCValue pushOntoStack(BCValue val)
+    {
+        if (!__ctfe) debug { import std.stdio; writeln("pushOntoStack: ", val); }


std usage

thewilsonator · 2022-06-25T01:12:48Z

src/dmd/ctfe/bc.d

+        {
+        case LongInst.SetHighImm32:
+            {
+                result ~= "SetHigh " ~ localName(stackMap, lw >> 16) ~ ", #" ~ itos(hi) ~ "\n";


this switch could be less cluttered with a local string function(string, uint) foo("SetHigh ",hi)

thewilsonator · 2022-06-25T01:15:57Z

src/dmd/ctfe/bc.d

+            }
+            break;
+
+        case LongInst.Jmp:


ditto for the jump instructions

thewilsonator · 2022-06-25T01:17:33Z

src/dmd/ctfe/bc.d

+__gshared int[ushort.max * 2] byteCodeCache;
+
+__gshared int byteCodeCacheTop = 4;


I do hope these are private... Also they could just be an int[]

thewilsonator

Review round for bc_common.d

thewilsonator · 2022-06-25T01:20:34Z

src/dmd/ctfe/bc_common.d

+}
+
+
+static assert(()


unittest?

unittests don't run at compile time ...

thewilsonator · 2022-06-25T01:22:18Z

src/dmd/ctfe/bc_common.d

+    {
+        BCTypeEnum commonType = commonTypeEnum(this.type.type, rhs.type.type);
+
+        if (this.vType == rhs.vType)


if ((this.vType != rhs.vType) return false;

thewilsonator · 2022-06-25T01:23:30Z

src/dmd/ctfe/bc_common.d

+    }
+}
+
+pragma(msg, "Sizeof BCValue: ", BCValue.sizeof);


put this behind debug conditional

thewilsonator · 2022-06-25T01:24:34Z

src/dmd/ctfe/bc_common.d

+}
+
+pragma(msg, "Sizeof BCValue: ", BCValue.sizeof);
+__gshared static immutable bcLastCond = () {


these do not need __gshared and they feel like they should be (static immutable) members of their types

thewilsonator · 2022-06-25T01:24:40Z

src/dmd/ctfe/bc_common.d

+    return result;
+}();
+
+__gshared static immutable bcNull = () {


thewilsonator · 2022-06-25T01:24:47Z

src/dmd/ctfe/bc_common.d

+__gshared static immutable bcFour = BCValue(Imm32(4));
+__gshared static immutable bcOne = BCValue(Imm32(1));
+__gshared static immutable bcZero = BCValue(Imm32(0));
+__gshared static immutable i32Type = BCType(BCTypeEnum.i32);
+__gshared static immutable u32Type = BCType(BCTypeEnum.u32);


thewilsonator · 2022-06-25T01:25:14Z

src/dmd/ctfe/bc_common.d

+
+BCTypeEnum commonTypeEnum(BCTypeEnum lhs, BCTypeEnum rhs) pure @safe
+{
+    // HACK


thewilsonator · 2022-06-25T01:26:42Z

src/dmd/ctfe/bc_common.d

+    bool ifTrue;
+}
+
+const(uint) align4(const uint val) @safe pure @nogc


these first couple of functions feel like they should be part of root or common, the statics should probably be private.

maxhaton · 2022-06-25T15:14:51Z

The whole fpconv module is probably going to be deleted

thewilsonator

Review round for bc_interpreter.

General comments: lots of std usage for debug.

thewilsonator · 2022-06-25T23:42:35Z

src/dmd/ctfe/bc_interpreter.d

+    {
+            if (cRetval.vType == BCValueType.Exception)


assert(cRetval.vType == BCValueType.Exception); and remove the massive indentation

thewilsonator · 2022-06-25T23:42:54Z

src/dmd/ctfe/bc_interpreter.d

+
+                // we return to unroll
+                // lets first handle the case in which there are catches on the catch stack
+                if (catches.length)


use early return here

thewilsonator · 2022-06-25T23:43:16Z

src/dmd/ctfe/bc_interpreter.d

+
+    bool Return()
+    {
+        if (n_return_addrs)


thewilsonator · 2022-06-25T23:46:57Z

src/dmd/ctfe/bc_interpreter.d

+            break;
+        case LongInst.ImmEq:
+            {
+                if ((*lhsStackRef) == imm32c)


cond = (*lhsStackRef) == imm32c);, ditto for the cases that follow this

thewilsonator · 2022-06-25T23:48:27Z

src/dmd/ctfe/bc_interpreter.d

+                uint _lhs = *lhsRef & uint.max;
+                float flhs = *cast(float*)&_lhs;
+                uint _rhs = *rhs & uint.max;
+                float frhs = *cast(float*)&_rhs;


make this a templated local function, templated on (T=={float, double} and make these cases shorter

thewilsonator · 2022-06-25T23:57:35Z

src/dmd/ctfe/bc_interpreter.d

+                if (Return()) return cRetval;
+            }
+            break;
+        case LongInst.RetS64:


thewilsonator · 2022-06-25T23:58:33Z

src/dmd/ctfe/bc_interpreter.d

+            {
+                if (*rhs == 0 && *lhsRef == 0)
+                {
+                    *lhsStackRef = 0;


use an early break

thewilsonator · 2022-06-25T23:58:55Z

src/dmd/ctfe/bc_interpreter.d

+                    const lhs_length = _lhs ? loadu32(llbasep) : 0;
+                    const rhs_length = _rhs ? loadu32(rlbasep) : 0;
+
+                    if (const newLength = lhs_length + rhs_length)


thewilsonator · 2022-06-26T00:00:08Z

src/dmd/ctfe/bc_interpreter.d

+                assert(_lhs && _rhs, "trying to deref nullPointers");
+                if (_lhs == _rhs)
+                {
+                    cond = true;


early break

thewilsonator · 2022-06-26T00:01:29Z

src/dmd/ctfe/bc_interpreter.d

+        }
+    }
+
+    static assert(testFact().interpret([imm32(5)]) == imm32(120));


this fails with

src\dmd\ctfe\bc_interpreter.d(1697): Error: function `dmd.ctfe.bc_interpreter.interpret(ref BCGen gen, BCValue[] args, BCHeap* heapPtr = null)` is not callable using argument types `(BCGen, BCValue[])` src\dmd\ctfe\bc_interpreter.d(1697): cannot pass rvalue argument `testFact()` of type `BCGen` to parameter `ref BCGen gen` src\dmd\ctfe\bc_interpreter.d(1697): while evaluating: `static assert(interpret(testFact(), [imm32(5u, false)]) == imm32(120u, false))`

expressions fire for -v and compile time (-debug=bc) respectively.

maxhaton added the Trivial typos, formatting, comments label Jun 23, 2022

dlang-bot added the Needs Work label Jun 23, 2022

thewilsonator reviewed Jun 25, 2022

View reviewed changes

ljmf00 removed the Trivial typos, formatting, comments label Jun 25, 2022

maxhaton added the Trivial typos, formatting, comments label Jun 25, 2022

thewilsonator reviewed Jun 26, 2022

View reviewed changes

dlang-bot added the Needs Rebase label Jul 10, 2022

UplinkCoder and others added 19 commits July 14, 2022 22:07

Add lastest version of newCTFE

2392bc1

Seperate out core.reflect extension

102fd0e

Prepare for proper handling of aggregate literals with runtime values

eab9c6c

Get rid of some writeln calls

5df35b0

Merge conflict

e9acdb9

fix arrayLiteral code

b1707b2

print DestroyTemporary with the semicolon

6e3b10b

Get better about the framePointer

46f21e9

Get Better about &x

3b81fda

Unifiy handling of paramters with how all other stack values are handled

351ade4

keep track of the last comment and fix emission of comments

a0014b6

change capacity allocation to allocate 1.3x instead of 32x

14669dc

Update newCTFE code to respect new EXP from TOK split

4b3be36

Remove some vestigial code from implementation of nodeFromName

99b2581

Remove vestigial backends

d5d16b4

Move the core interpreter into it's own file

1aca0a1

Make logging output for function execution and Symbol offset

91ab78e

expressions fire for -v and compile time (-debug=bc) respectively.

Make top-level module documentation consistent with the rest of dmd.

801b502

A bit more detail for bc.d and the interpreter

7dc6503

maxhaton added 3 commits July 14, 2022 22:07

Add a scope

ecb907a

Shut pre-commit up

8380f39

Rebase atop of the nu structure.

99044e6

maxhaton force-pushed the tryRebaseCTFE branch from ba1b850 to 99044e6 Compare July 14, 2022 21:13

dlang-bot removed Needs Work Needs Rebase labels Jul 14, 2022

maxhaton added 2 commits July 14, 2022 22:17

Move fpconv into dmd.common

d865e41

Add module docs for fpconv

2a0160b

dlang-bot added the Needs Work label Jul 15, 2022

dlang-bot added the Needs Rebase label Aug 4, 2022

dlang-bot added the stalled label Sep 23, 2022

Merge branch 'master' into tryRebaseCTFE

717764e

dlang-bot removed Needs Work Needs Rebase stalled labels Jun 28, 2023

Update cli.d

8acba3c

dlang-bot added Needs Work stalled labels Jun 28, 2023

dlang-bot added the Needs Rebase label Nov 22, 2023

		static assert(!bc_order_errors.length, bc_order_errors);


		pragma(msg, 2 ^^ 7 - LongInst.max, " opcodes remaining");

		__gshared int[ushort.max * 2] byteCodeCache;

		__gshared int byteCodeCacheTop = 4;

newCTFE rebase-dump #14243

Are you sure you want to change the base?

newCTFE rebase-dump #14243

Conversation

maxhaton commented Jun 23, 2022

dlang-bot commented Jun 23, 2022

Bugzilla references

Testing this PR locally

thewilsonator left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thewilsonator Jun 25, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thewilsonator left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thewilsonator left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxhaton commented Jun 25, 2022

thewilsonator left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thewilsonator Jun 25, 2022 •

edited