Skip to content
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.

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

LDC-specific _d_array_slice_copy (betterC) #2425

Open
deviator opened this issue Nov 22, 2017 · 29 comments
Open

LDC-specific _d_array_slice_copy (betterC) #2425

deviator opened this issue Nov 22, 2017 · 29 comments

Comments

@deviator
Copy link

Hello. I try build this simple example:

import core.stdc.stdio;
import std.algorithm : min;

extern (C) void main()
{
    char[256] buf;
    buf[] = '\0';

    auto str = "hello world";
    auto ln = min(buf.length, str.length);
    buf[0..ln] = str[0..ln];
    printf("%s\n", buf.ptr);
}

ldc2 -betterC bettercarray2.d

And I get error:

bettercarray2.o: In function `main':
bettercarray2.d:(.text.main[main]+0xd0): undefined reference to `_d_array_slice_copy'
collect2: error: ld returned 1 exit status
Error: /usr/bin/gcc failed with status: 1

On dmd 2.076.1 I have same problem, but it solves by -noboundscheck flag.
If I try -boundscheck=off on ldc2 I have no effect.

ldc2 version is 1.6.0-beta1 (prebuild for linux x86_64)

@kinke
Copy link
Member

kinke commented Nov 22, 2017

As the name of the function suggests, this isn't directly related to array bounds checks, but with buf[0..ln] = str[0..ln] being mapped to druntime function _d_array_slice_copy() (which probably also checks that both slices feature the same length). We'll have to check what DMD does in this case; it's most likely unrelated to -betterC.

@kinke
Copy link
Member

kinke commented Nov 22, 2017

See https://github.com/ldc-developers/druntime/blob/ldc/src/ldc/arrayinit.d#L142. According to the comment, it's only used for enabled assertions (also checking that the slices don't overlap); so an additional -release makes it work: https://godbolt.org/g/joZVzt

@deviator
Copy link
Author

We'll have to check what DMD does in this case; it's most likely unrelated to -betterC.

It related to -betterC if it try use runtime when code builds with -betterC flag. -betterC must don't use runtime and all calls must be replaced with inline code or something else (c-runtime equivalent for example). But it's behavior isn't related to ldc, it's must be implement in dmd frontend...

This issue about difference between behavior of ldc and dmd.

@kinke
Copy link
Member

kinke commented Nov 22, 2017

It related to -betterC if it try use runtime when code builds with -betterC flag.

This divergence between LDC and DMD has absolutely nothing to do with -betterC. It just happens to be noticed with -betterC.

But it's behavior isn't related to ldc, it's must be implement in dmd frontend...

Well, there's not a single -betterC specific special case in the front-end right now. I.e., Walter's semantics have to be reimplemented by each compiler.

-betterC is a mess right now and probably will be for some time, if it ever gets to a point of really being useful for more wide-spread real-world code. What's your motivation for getting rid of druntime? Just curious as I don't understand the motivation unless one is targeting bare metal and/or extremely resource-constrained systems.

@kinke kinke changed the title betterC tried check array bounds LDC-specific _d_array_slice_copy (betterC / inlineability) Nov 22, 2017
@kinke kinke changed the title LDC-specific _d_array_slice_copy (betterC / inlineability) LDC-specific _d_array_slice_copy (betterC) Nov 22, 2017
@kinke
Copy link
Member

kinke commented Nov 22, 2017

[Inlineability seems not to be a real issue as _d_array_slice_copy is optimized by the SimplifyDRuntimeCalls pass, which is enabled starting with -O2.]

Enabling that pass generally for -betterC seems like a good idea, it should prevent a few other druntime dependencies as well.

@PetarKirov
Copy link
Contributor

PetarKirov commented Nov 23, 2017

@kinke what's the interaction between SimplifyDRuntimeCalls pass and the new templated druntime builtins, like the forthcoming (for LDC 1.7) https://github.com/dlang/druntime/blob/master/src/core/internal/arrayop.d#L28 ?

IMO, SimplifyDRuntimeCalls should handle the non-inline-able non-templated built-ins. With the end goal that after every built-in becomes a template function (probably quite far off from now) there should be no need for this pass on the LDC side.

@kinke
Copy link
Member

kinke commented Nov 23, 2017

No interaction at all, as it handles no arrayops, see https://github.com/ldc-developers/ldc/blob/master/gen/passes/SimplifyDRuntimeCalls.cpp#L344-L369.

@deviator
Copy link
Author

@kinke

What's your motivation for getting rid of druntime? Just curious as I don't understand the motivation unless one is targeting bare metal and/or extremely resource-constrained systems.

I work in company that develop devices for industrial mainly for uninterruptible power source (monitoring, measurement etc). Some devices we develop on RPI, some on other ARM-boards, some devices fully develop by my colleagues and based on STM32, MSP430 etc. Last one use C for firmware. I try use D on ARM-board devices (with linux on the board) but have this problem. This is main blocker for using D on future projects on ARM-boards (impossible to determine place there throwing exception will lead to this error). Now I try eliminate druntime to get more predictable behavior. If -betterC mode will be finalized and stable I can try it for firmware.

@kinke
Copy link
Member

kinke commented Nov 23, 2017

You do realize there is no exception support whatsoever with -betterC?

So if you don't need them, you won't run into the EH issue anyway.

@deviator
Copy link
Author

You do realize there is no exception support whatsoever with -betterC?

Yes, I know.

So if you don't need them, you won't run into the EH issue anyway.

Druntime can throws. For minimize probability of throwing I must check all data before any druntime call. And as a result, in any case I must put all druntime call in try-catch block. Or rewrite interested functions by myself as nothrow. In the end than it differs from -betterC by labor costs? With druntime I have probability of fatal error (I can't it catch), I have necessity to write many verification code (as without druntime), and in some cases need rewrite functions (as without druntime). If EH will works good I will not think about eliminate druntime.

@kinke
Copy link
Member

kinke commented Nov 23, 2017

Well, avoiding druntime is what needs to be done for -betterC anyway. The only thing which works out of the box so far are asserts as they map to the C assert in betterC mode. So -betterC doesn't magically solve your problem.

If you're so worried about hitting the EH issue, why don't you try disabling inlining as Joakim suggested in your issue? Or is your code performance-critical as well and that's really not an option?

@joakim-noah
Copy link
Contributor

Even if performance might be an issue, he should still try disabling inlining to see if that's causing the problem.

@deviator
Copy link
Author

At this moment I don't know where I can get EH problem, because compile-to-compile place changes. I include -disable-inlining to ldc2.conf.

@joakim-noah
Copy link
Contributor

If you can't figure out the problem, my advice is to get one of the ldc devs under an NDA and let them get their hands on it, for a consulting fee. My understanding is that some of them do this for Weka. It sounds like you will save a lot of your time and money rather than trying to do everything yourself.

@ryuukk
Copy link
Contributor

ryuukk commented Feb 26, 2021

What's the progress on this? it's been 4 years already, and i got hit by this issue

@deviator
Copy link
Author

What's the progress on this? it's been 4 years already, and i got hit by this issue

original problem not solved in ldc 1.24.0 (with _d_array_slice_copy), but my problem with strange exceptions what motivate to try betterC are solved far ago (in next compiler version) and I successful continue develop for ARM linux on D with runtime and GC

@ryuukk
Copy link
Contributor

ryuukk commented Mar 8, 2021

@deviator ok thanks

That is the only issue that prevents me from using -betterC, it works fine with DMD, but not with LDC

@kinke
Copy link
Member

kinke commented Mar 8, 2021

@ryuukk
Copy link
Contributor

ryuukk commented Mar 8, 2021

See https://forum.dlang.org/post/heksucpdamkgwnztyitr@forum.dlang.org for the workaround.

@kinke Thanks, the workaround works!

Do you have an idea about this one? it works with DMD, so i guess it is a similar issue as the copy one

    char[] ext = extbuf[0..extlen];
    switch (ext[0]) {
        case 't': if (ext == "tga") return IF_TGA; else return -1;
        case 'b': if (ext == "bmp") return IF_BMP; else return -1;
        case 'p': if (ext == "png") return IF_PNG; else return -1;
        case 'j': if (ext == "jpg" || ext == "jpeg") return IF_JPG; return -1;
        default: return -1;
    }

game.obj : error LNK2019: unresolved external symbol _D4core8internal5array8equality__T8__equalsTaTaZQoFNaNbNiNeMxAaMxQeZb referenced in function _D4dawn5image9fname2fmtFNbNiIAaZi

@kinke
Copy link
Member

kinke commented Mar 8, 2021

This looks template-culling related; try with -linkonce-templates.

@ryuukk
Copy link
Contributor

ryuukk commented Mar 8, 2021

@kinke it works! thanks!!

Is this a bug? should it be reported somewhere?

@kinke
Copy link
Member

kinke commented Mar 9, 2021

Nope, this is all to be expected with current -betterC. It simply assumes you're not going to use anything from the prebuilt druntime library, so missing compiler helper functions for array copying (incl. matching length checks etc., much better suited as library code than laboriously and intransparently inlining code from the compiler directly) etc. will result in these linker errors. Templates from druntime work in some/most? cases, unless the template culling algorithm kicks in, fully unaware of any -betterC and that druntime won't be linked.

Making _d_array_slice_copy a template might work around it, but in the end just add up to the pile of -betterC hacks scattered all over the place.

@ryuukk
Copy link
Contributor

ryuukk commented Mar 9, 2021

Oh i see, i wish there was an easier solution

I will try to document all the issues i had and their solution, so at least it'll be easier for people in similar situation as me

EDIT: here is a first draft: https://gist.github.com/ryuukk/e0b40012dcde9f071bf414ecec9c9ccf

@ryuukk
Copy link
Contributor

ryuukk commented Jul 24, 2022

I got hit by the issue again and i didn't remember about the workaround..

I think the template hack you mentioned is worth it if users gets a cleaner experience as a result

I don't see myself carying that function with me whenever i create a project

lld-link: error: undefined symbol: _d_array_slice_copy

image

feels bad

@ryuukk
Copy link
Contributor

ryuukk commented Feb 17, 2023

Any news on this?

I'm tired of having to include that file for all my projects

If someone could indicate me where in the code base the problem lies, i could try to come up with a PR

If the issue is known and a fix is possible but is not desirable, then i'd love a patch so i can maintain a private fork

@kinke
Copy link
Member

kinke commented Feb 17, 2023

This should 'work', skipping the bounds and overlap checks for -betterC even though the cmdline flags imply those checks:

diff --git a/gen/arrays.cpp b/gen/arrays.cpp
index 14b1b26614..bbc24062c2 100644
--- a/gen/arrays.cpp
+++ b/gen/arrays.cpp
@@ -164,7 +164,8 @@ static void copySlice(const Loc &loc, LLValue *dstarr, LLValue *dstlen,
                       LLValue *srcarr, LLValue *srclen, size_t elementSize,
                       bool knownInBounds) {
   const bool checksEnabled =
-      global.params.useAssert == CHECKENABLEon || gIR->emitArrayBoundsChecks();
+      !global.params.betterC && (global.params.useAssert == CHECKENABLEon ||
+                                 gIR->emitArrayBoundsChecks());
   if (checksEnabled && !knownInBounds) {
     LLFunction *fn = getRuntimeFunction(loc, gIR->module, "_d_array_slice_copy");
     gIR->CreateCallOrInvoke(

@ryuukk
Copy link
Contributor

ryuukk commented Feb 18, 2023

I will give this a try, thanks a lot

@ryuukk
Copy link
Contributor

ryuukk commented Apr 17, 2023

Wait, i missed an important detail

You said: "skipping the bounds and overlap checks"

Why would someone want to skip that? i want that in debug build, did i misunderstand the feature?

@kinke
Copy link
Member

kinke commented Apr 17, 2023

These checks are the only reason why a druntime function is called - asserting that the array lengths match and the arrays don't overlap, constructing a nice assertion msg (incl. the array lengths etc.) with the GC if violated. If similar checks are desired for betterC - well, then some external implementation not depending on the GC is required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants