[for information only] LDC diff with DMD: 2.077.1 by JohanEngelen · Pull Request #7506 · dlang/dmd

JohanEngelen · 2017-12-24T13:26:12Z

Copy-paste of LDC master 24 Dec (8c0abd5220e4993d66336b57742c1988dfd73d3f) onto DMD 2.077.1

Continuation of #4688

Note: parts of the diff have already been applied to DMD master.

…3d3f) onto DMD 2.077.1

dlang-bot · 2017-12-24T13:26:12Z

Thanks for your pull request, @JohanEngelen! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

smaller, focused PRs are easier to review than big ones
try not to mix up refactoring or style changes with bug fixes or feature enhancements
provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

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.

ibuclaw

There's an awful lot here, and I gave up looking at it. :-)

ibuclaw · 2017-12-24T13:31:32Z

src/ddmd/aggregate.h

    void semanticTypeInfoMembers();
    Dsymbol *search(Loc, Identifier *ident, int flags = SearchLocalsOnly);
-    const char *kind();
+    const char *kind() const;


ibuclaw · 2017-12-24T13:32:54Z

src/ddmd/attrib.d

            }
            return createNewScope(sc, sc.stc, sc.linkage, sc.cppmangle, sc.protection, sc.explicitProtection, sc.aligndecl, inlining);
        }
+        else if (IN_LLVM && ident == Id.LDC_profile_instr) {


Propose a way to add pragmas to the frontend via a hook. (Maybe #7457 might be the place to put it).

ibuclaw · 2017-12-24T13:33:21Z

src/ddmd/attrib.h

    void addMember(Scope *sc, ScopeDsymbol *sds);
    void setScope(Scope *sc);
    void importAll(Scope *sc);
+    void semantic(Scope *sc);


Why? Semantic passes should have been removed and visitor()-ized.

It's just a merge artefact – benign, as the function is not virtual, but fixed in master nevertheless.

ibuclaw · 2017-12-24T13:33:34Z

src/ddmd/attrib.h

    Dsymbols *include(Scope *sc, ScopeDsymbol *sds);
    void addMember(Scope *sc, ScopeDsymbol *sds);
-    void addComment(const char *comment);
+    void addComment(const utf8_t *comment);


Wasn't this committed recently?

ibuclaw · 2017-12-24T13:35:02Z

src/ddmd/builtin.d

+version(IN_LLVM)
+{
+
+private Type getTypeOfOverloadedIntrinsic(FuncDeclaration fd)


Maybe same as per comment on pragmas. Need a way to extend front-end built-ins for each compiler (although gdc just ignores this module).

ibuclaw · 2017-12-24T13:36:40Z

src/ddmd/constfold.d

+        // if there is no offset. With an offset, we can at least say that it is
+        // non-zero.
+        SymOffExp soe = cast(SymOffExp) e;
+        if (soe.var.llvmInternal == LDCPragma.LLVMextern_weak && !soe.offset)


Bug / test for this? Seems innocuous.

This is related to an LDC-only pragma, LDC_extern_weak, which can be used to implement ELF-style weak symbols (on targets that support them). The frontend would constant-fold away the null checks to see whether they are present, since it assumed that symbol addresses are always non-zero (the 2 below). There is a regression test for it in our lit test suite.

(In fact, the pragma was never publicly documented, as I wanted to have something portable across the compilers instead, but I needed it to implement shared libraries in a way that is compatible with --gc-sections due to a certain ld.bfd bug. If the pragma was added upstream, this hunk would also apply.)

ibuclaw · 2017-12-24T13:37:41Z

src/ddmd/dcast.d

+                {
+                    // Cast the CommaExp instead of the inner rhs; this fixes
+                    // https://github.com/ldc-developers/ldc/issues/2415.
+                    result = new CastExp(e.loc, e, e2c.type);


Can't you handle this properly in code-gen?

ibuclaw · 2017-12-24T13:38:11Z

src/ddmd/declaration.h

+
+    // Whether to emit instrumentation code if -fprofile-instr-generate is specified,
+    // the value is set with pragma(LDC_profile_instr, true|false)
+    bool emitInstrumentation;


Can't you do heuristics at code-gen level?

ibuclaw · 2017-12-24T13:38:48Z

src/ddmd/dmodule.d

+                errorSupplemental(loc, "ldc2 might not be correctly installed.");
+                errorSupplemental(loc, "Please check your ldc2.conf configuration file.");
+                errorSupplemental(loc, "Installation instructions can be found at http://wiki.dlang.org/LDC.");
+}


We need a way to have a per-compiler message here, again with Compiler hook.

ibuclaw · 2017-12-24T13:39:15Z

src/ddmd/dmodule.d

+        // Coverage analysis
+        void* d_cover_valid;  // llvm::GlobalVariable* --> private immutable size_t[] _d_cover_valid;
+        void* d_cover_data;   // llvm::GlobalVariable* --> private uint[] _d_cover_data;
+        Array!size_t d_cover_valid_init; // initializer for _d_cover_valid


Can't you attach this data to LLVM AST?

skl131313 · 2017-12-24T17:56:17Z

Why is this being merged into dmd? Unless tests get added for it, it'll constantly be broken and just adds bloat otherwise.

JinShil · 2017-12-24T20:49:05Z

@skl131313 It's just informational. It is not intended to be merged.

WalterBright · 2017-12-24T22:41:59Z

@ibuclaw @JohanEngelen Since dmd doesn't use the C++ .h files anymore, I'll defer the merging of them to you guys.

ibuclaw · 2017-12-24T23:10:20Z

Since dmd doesn't use the C++ .h files anymore, I'll defer the merging of them to you guys.

Well, any changes to struct/class layout must also be reflected on the D side too. Otherwise there'd be linker problems, or worse, runtime corruption.

andralex · 2018-01-20T18:30:50Z

So what's the story here? There seems to be a lot of good stuff here but it's in a form that's difficult to review.

JinShil · 2018-03-23T02:10:35Z

@JohanEngelen Has this PR served its purpose? Can you update it, or can we close it?

Copy-paste of LDC master 24 Dec (8c0abd5220e4993d66336b57742c1988dfd7…

86f4563

…3d3f) onto DMD 2.077.1

JohanEngelen mentioned this pull request Dec 24, 2017

Current LDC frontend diff #4688

Closed

JohanEngelen changed the title ~~LDC diff with DMD~~ LDC diff with DMD: 2.077.1 Dec 24, 2017

ibuclaw reviewed Dec 24, 2017

View reviewed changes

JinShil added the Compiler:LDC LLVM based D Compiler label Dec 24, 2017

dlang-bot added Review:Needs Rebase Review:Needs Work labels Jan 1, 2018

dnadlinger added Review:WIP Work In Progress - not ready for review or pulling and removed Review:Needs Rebase Review:Needs Work labels Jan 2, 2018

dlang-bot added Review:Needs Rebase Review:Needs Work labels Jan 3, 2018

JohanEngelen changed the title ~~LDC diff with DMD: 2.077.1~~ [for information only] LDC diff with DMD: 2.077.1 Jan 20, 2018

JohanEngelen closed this Mar 23, 2018

WalterBright added the Review:Phantom Zone Has value/information for future work, but closed for now label Mar 23, 2018

Uh oh!

Conversation

JohanEngelen commented Dec 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dlang-bot commented Dec 24, 2017

Bugzilla references

Uh oh!

ibuclaw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dnadlinger Dec 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skl131313 commented Dec 24, 2017

Uh oh!

JinShil commented Dec 24, 2017

Uh oh!

WalterBright commented Dec 24, 2017

Uh oh!

ibuclaw commented Dec 24, 2017

Uh oh!

andralex commented Jan 20, 2018

Uh oh!

JinShil commented Mar 23, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

JohanEngelen commented Dec 24, 2017 •

edited

Loading

dnadlinger Dec 25, 2017 •

edited

Loading