Clean up druntime fwd decls

[JohanEngelen]

WORK IN PROGRESS

Makes the code much prettier I hope.
The main motivation is to enable sret for dynamic arrays, see closed PR #901.

[kinke]

Attributes index 1 is most likely wrong (1st LL parameter). You'll probably want llvm::AttributeSet::AttrIndex::FunctionIndex = ~0U for the function attributes and make sure that addAttributes() merges it with potential existing ones.

[kinke]

The main motivation is to enable sret for dynamic arrays

More precisely: passing (incl. sret) dynamic arrays (slices) as regular 16-bytes struct for the Win64 ABI. ;)
Looks way better already. I guess you can workaround the fnames initializer list issue for LLVM < 3.7 by declaring it as llvm::ArrayRef<const char *> instead.

[JohanEngelen]

(I actually only have half a clue what "sret" means, lol :D)

[kinke]

https://github.com/ldc-developers/ldc/blob/master/gen/abi.h#L100 hopefully explains it, and https://github.com/ldc-developers/ldc/blob/master/gen/abi-generic.h#L180 should clarify why we need an ABIRewrite for dynamic arrays on Win64.

[dnadlinger]

While you are at it, could you please get rid of the LLVM_D_ prefix thing? It's not used anywhere else. I think our plan is to use ldc::, maybe ldc::runtime:: for this.

[JohanEngelen]

OK, will do. Saving symbol name cleanup for last (note all the dumb "DD" prefixing for now).

[JohanEngelen]

Unfortunately, LLVM3.5 has initializer list constructor of ArrayRef behind an #ifdef LLVM_HAS_INITIALIZER_LISTS. That define is really only set for Clang…

Edit: Sry Johan, I've edited your comment instead of adding my own (kinke). I don't see how I can restore it. :/

[dnadlinger]

Thanks for tackling the naming cleanup. I'm wondering, by the way, whether we should really keep the separate runtime module, or whether it would be a better idea to just lazily create them in the real codegen modules. There doesn't seem to be any relevant extra overhead, if you are compiling a gazillion small D modules all at once, you'll run into other limitations first.

-DLLVM_HAS_INITIALIZER_LISTS

Seems like a sensible workaround.

[kinke]

-DLLVM_HAS_INITIALIZER_LISTS

Or std::vector<llvm::StringRef>. There'll be a lot more ArrayRefs anyway, and we all like initializer lists. ;)

[dnadlinger]

We do know that all our target compilers support C++11, so I don't mind working around the LLVM issue at its source.

[JohanEngelen]

I added a semi-overkill solution for ArrayRef :)
OK, added -DLLVM_HAS_INITIALIZER_LISTS to compile flags
Should have looked a little longer at LLVM's code. LLVM_HAS_INITIALIZER_LISTS is actively suppressed by the Support/Compiler.h header. So -D... won't work. For now, I think I'll go for the previous 'overkill' solution.

[JohanEngelen]

(rebased to hide embarrassing coding error ;))

[JohanEngelen]

@kinke I'm sorry I am sometimes flooding the AppVeyor build server with a string of new commits when trying to debug an issue that perhaps I don't see locally. Is there a way to cancel or remove jobs from the queue? (On Travis, I can click the "cancel job" button)

[kinke]

Hehe no worries. Unfortunately I have no idea how to give you guys proper access to the AppVeyor project. I just set some explicit permissions for the user role, but I haven't figured how to map GitHub accounts to roles. We may need an official ldc-developer account for AppVeyor.

[JohanEngelen]

@kinke @klickverbot I think the clean-up is ready. What do you think?
(Next step is to move to Windows and see what breaks when I enable passing of dyn arrays as regular structs. What I remember from last time is that the codegen code assumes too much about the function signatures and will break when the Windows ABI changes.)

[kinke]

This should be replaceable by

fn->setAttributes(AttrSet(fn->getAttributes()).merge(attribset));

with #1217.

[JohanEngelen]

thanks!

[kinke]

Just one more thing - please convert this to a method of IrFuncTy, as it nearly exclusively depends upon it.

[JohanEngelen]

It's still not super pretty. It's because IrFuncTy apparently does not know whether to passThisBeforeSret. Fixing it would require more surgery though :(

[kinke]

Lgtm, thanks Johan.

So the ABI rewrites are now applied when forward declaring all runtime functions [except for _d_dso_registry() on Linux, which is declared directly in IR].
That doesn't seem to have any effect so far, otherwise we'd already have the problem that we are facing next: applying the ABI rewrites for return value and arguments when calling a runtime function. For regular functions, that's all in DtoCall() and mainly depends on IrFuncTy. So we can't call the LLFunction returned by getRuntimeFunction() directly at IR level anymore.

[JohanEngelen]

@thanks kinke.
Indeed, now we have to add a "callRuntimeFunction()" method I think.