convert blockopt.c to blockopt.d #8426

wilzbach · 2018-07-01T19:21:42Z

No description provided.

dlang-bot · 2018-07-01T19:21:43Z

Thanks for your pull request, @wilzbach!

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 fetch digger
dub run digger -- build "master + dmd#8426"

wilzbach · 2018-07-01T19:25:11Z

src/dmd/backend/blockopt.d

+    vec_free(b.Bgen2);
+    vec_free(b.Bkill2);
+
+    memset(&b,0,b.sizeof);


I'm not sure whether this is the correct translation.

It isn't. It's supposed to zero the pointers to the vectors that were just freed. An easy way to do this is to write a nested function:

static void vfree(ref vec_t* v) { vec_free(v); v = null; }

and call that instead of vec_free.

wilzbach · 2018-07-01T19:27:21Z

src/dmd/backend/blockopt.d

-                    b2->BC = BCret;             /* a harmless one       */
-                    cmes3("brcombine(): %p goto %p eliminated\n",b,b2);
+                    b.Bswitch = b2.Bswitch;
+                    b.Bswitch = b2.Bswitch;


I'm not sure whether this is the correct translation.

This is copying too little, it should be somethink llike

b.BC = b2.BC; b.cover_BS[] = b2.cover_BS[];

with

// add member at least as large as the other elements of this union void[jcatchvar.sizeof + Bscope_index.sizeof + Blast_index.sizeof] cover_BS;

added to the respective union in block.

wilzbach · 2018-07-01T19:31:15Z

src/dmd/backend/blockopt.d

+                    b2.Bnext = b.Bnext;
+                    b.Bnext = b2;
+                    b2.BC = b.BC;
+                    b2.Bswitch = b.Bswitch;


I'm not sure whether this is the correct translation.

jacob-carlborg · 2018-07-02T05:49:03Z

src/dmd/backend/blockopt.d


 ////////////////////////////
 // Storage allocator.

-static block blkzero;


Walter usually coverts static to private.

This is in a private __gshared block.

wilzbach · 2018-07-05T06:11:48Z

Not sure why Windows isn't passing.
Auto-tester simply times out for Windows when building sanitize_json:

timed out after 3600 seconds, step failed

and AppVeyor even fails while linking:

backend.lib(blockopt.obj) : error LNK2001: unresolved external symbol _D3dmd7backend5gflow12__ModuleInfoZ
``

rainers · 2018-07-05T06:51:27Z

and AppVeyor even fails while linking:

backend.lib(blockopt.obj) : error LNK2001: unresolved external symbol _D3dmd7backend5gflow12__ModuleInfoZ

This happens because some files are compiled with -betterC, but blockopt is not. I suspect some of the import dependencies have static this().

rainers · 2018-07-06T08:22:31Z

src/dmd/backend/blockopt.d


        /* Search for two blocks that have the same successor list.
           If the first expressions both lists are the same, split
           off a new block with that expression in it.
         */
-        for (block *b = startblock; b; b = b->Bnext)
+        static if (SCPP_OR_NTEXCEPTIONS)
+            bool additionalAnd = b.Btry == bn.Btry;


This seems not to be actually compiled, but it has to be inside the inner loop.

rainers · 2018-07-06T08:52:37Z

Not sure why Windows isn't passing.
Auto-tester simply times out for Windows when building sanitize_json:

It's not the building of sanitize_json, but the execution of the d_do_test unittests: it waits forever for threads to join when it returns from main. I guess there are still code gen bugs (after my proposed changes).

rainers · 2018-07-06T17:44:36Z

handling of NTEXCEPTIONS was incorect, too. I pushed some fixes.

wilzbach · 2018-07-09T07:52:37Z

I pushed some fixes.

Wow. Thanks a lot! It's still failing in the Visual Studio build with:

  ..\dmd\backend\blockopt.d(55): error : Function type does not match previously declared function with the same mangled name: `mem_ffree` [C:\projects\dmd\src\vcbuild\dmd.vcxproj]

I don't see where it's apparently declared previously...

rainers · 2018-07-11T06:01:43Z

I don't see where it's apparently declared previously...

This is merge conflict because #8466 now also defines mem_ffree.

wilzbach · 2018-07-16T16:09:59Z

BTW this is passing the CIs now :)

jacob-carlborg · 2018-07-17T05:40:02Z

BTW this is passing the CIs now :)

No 😊. Merge conflicts.

fix handling NTEXCCEPTIONS

wilzbach · 2018-07-17T10:56:08Z

No blush. Merge conflicts.

Resolved.

WalterBright

Nice work! Fix the memset as described, and it's good to go.

wilzbach requested a review from MartinNowak as a code owner July 1, 2018 19:21

wilzbach force-pushed the blockopt branch 3 times, most recently from 67bb9e1 to 9110a65 Compare July 1, 2018 19:24

wilzbach commented Jul 1, 2018

View reviewed changes

wilzbach force-pushed the blockopt branch from 9110a65 to bcc28d9 Compare July 1, 2018 19:42

wilzbach requested review from ibuclaw and dnadlinger as code owners July 1, 2018 19:42

wilzbach force-pushed the blockopt branch 2 times, most recently from d6eed7b to 61511fb Compare July 1, 2018 19:45

wilzbach added the D Conversion label Jul 1, 2018

wilzbach force-pushed the blockopt branch from 61511fb to 8d9a18d Compare July 1, 2018 20:12

jacob-carlborg reviewed Jul 2, 2018

View reviewed changes

wilzbach force-pushed the blockopt branch 5 times, most recently from 38b92ea to 0f086ef Compare July 5, 2018 06:08

wilzbach force-pushed the blockopt branch from 0f086ef to 6c05085 Compare July 5, 2018 23:18

wilzbach requested review from Geod24, rainers and WalterBright as code owners July 6, 2018 03:23

rainers reviewed Jul 6, 2018

View reviewed changes

wilzbach force-pushed the blockopt branch from 494fa9a to ec95db3 Compare July 6, 2018 08:45

wilzbach force-pushed the blockopt branch from 711ed26 to 8111e2f Compare July 9, 2018 01:47

wilzbach force-pushed the blockopt branch from 8111e2f to 82e9663 Compare July 9, 2018 07:49

wilzbach force-pushed the blockopt branch from 82e9663 to a5ae174 Compare July 9, 2018 09:35

wilzbach force-pushed the blockopt branch 2 times, most recently from 6e802bd to 16be93f Compare July 11, 2018 09:47

wilzbach and others added 2 commits July 17, 2018 12:54

convert blockopt.c to blockopt.d

23fb520

fix copying block.BS

90dd254

fix handling NTEXCCEPTIONS

wilzbach force-pushed the blockopt branch from 16be93f to 60edaf4 Compare July 17, 2018 10:55

Add mem.h to the D object files

60edaf4

WalterBright requested changes Jul 17, 2018

View reviewed changes

Use vfree instead of memset

edee16c

WalterBright approved these changes Jul 18, 2018

View reviewed changes

WalterBright merged commit 31e7afa into dlang:master Jul 18, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

convert blockopt.c to blockopt.d #8426

convert blockopt.c to blockopt.d #8426

wilzbach commented Jul 1, 2018

dlang-bot commented Jul 1, 2018

wilzbach Jul 1, 2018

WalterBright Jul 17, 2018

wilzbach Jul 1, 2018

rainers Jul 6, 2018

wilzbach Jul 1, 2018

rainers Jul 6, 2018

jacob-carlborg Jul 2, 2018

wilzbach Jul 2, 2018

jacob-carlborg Jul 2, 2018

wilzbach commented Jul 5, 2018

rainers commented Jul 5, 2018

rainers Jul 6, 2018

rainers commented Jul 6, 2018

rainers commented Jul 6, 2018

wilzbach commented Jul 9, 2018 •

edited

Loading

rainers commented Jul 11, 2018

wilzbach commented Jul 16, 2018

jacob-carlborg commented Jul 17, 2018

wilzbach commented Jul 17, 2018

WalterBright left a comment

convert blockopt.c to blockopt.d #8426

convert blockopt.c to blockopt.d #8426

Conversation

wilzbach commented Jul 1, 2018

dlang-bot commented Jul 1, 2018

Bugzilla references

Testing this PR locally

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

wilzbach commented Jul 5, 2018

rainers commented Jul 5, 2018

Choose a reason for hiding this comment

rainers commented Jul 6, 2018

rainers commented Jul 6, 2018

wilzbach commented Jul 9, 2018 • edited Loading

rainers commented Jul 11, 2018

wilzbach commented Jul 16, 2018

jacob-carlborg commented Jul 17, 2018

wilzbach commented Jul 17, 2018

WalterBright left a comment

Choose a reason for hiding this comment

wilzbach commented Jul 9, 2018 •

edited

Loading