Fix gc2stack pass

[Hardcode84]

No description provided.

[Hardcode84]

druntime-test-exceptions/invalid_memory_operation.d runs code

struct S
{
    ~this()
    {
        new int;
    }
}

void main()
{
    new S;
}

and expects it will crash with InvalidMemoryOperationError.
This obviously won't happen after this optimization as new int is moved to stack and optimized out.

[dnadlinger]

Breaking that test is fine imho – iirc it was only supposed to test that there is no infinite recursion caused by the EH code or something along those lines.

[Hardcode84]

Fix for invalid_memory_operation test: ldc-developers/druntime#145

[kinke]

@Hardcode84: How did you manage to run those tests with enabled optimizations? Do you set the DFLAGS env variable manually?

[Hardcode84]

@kinke just run ctest from git bash terminal.

[kinke]

Well AFAICT that can't be without some special setup from your side, I mean this test hasn't been failing for CI and I've never seen it fail myself, plus I don't see any -O in the corresponding Makefile.

[Hardcode84]

@kinke They passed because gc2stack pass was completely broken for (probably) very long time.

[kinke]

Nope they passed because they are never run with -O and so no gc2stack.

[Hardcode84]

If I revert submodule update from this PR, they will fail on CI because of gc2stack as well

[kinke]

[I've been on vacation this week, so I'm not fully up-to-date.]

[Hardcode84]

@kinke

https://github.com/ldc-developers/druntime/blob/ldc/test/common.mak#L30

[kinke]

Alright, so the druntime standalone tests are (only) run with -O -release, and GC2Stack was really badly broken, only working for 1 of your new testcases (foo3). Thanks for the fix!

I'm planning on releasing 1.10 final these days; how's your pipeline looking, any more patches to come in? :)

[Hardcode84]

I'm not tied to releases and this is somewhat dangerous fix for final version. There can be cases where it will incorrectly optimize which I didn't foresee.

[Hardcode84]

BTW, I don't see release branch, do we release from master now?

[dnadlinger]

No point in keeping this around, right?

[Hardcode84]

removed

[Hardcode84]

@kinke You can merge this if you think it should go into 1.10 release, but I'm fine if it go to next release.

[kinke]

I'd have included it if it was just me, as it seems to make GC2Stack work as intended (but yeah, that might cause issues indeed). The next beta is only a couple of weeks away I guess, so let's wait a little bit with this one.

[Hardcode84]

Fun fact: there is also SimplifyDRuntimeCalls pass which should remove gc allocations completely if their result isn't used and break invalid_memory_operation test even if -disable-gc2stack specified.
But it didn't so it is probably also broken.

[kinke]

No more stripPointerCasts()?

[Hardcode84]

I can add it but I'm not sure how to trigger it.

Some context: first metadata operand is GlobalVariable for Typinfo object and we compare it with GlobalVariable typeinfo passed as parameter to allocation function.

[kinke]

[The TypeInfo globals used to be pointers to anonymous IR struct types IIRC; I changed them to the final TypeInfo subtypes (TypeInfo_Struct, TypeInfo_Class etc.).] So if the param is a TypeInfo* base class reference, the casts are better stripped for the comparison; they are already stripped for ti_global.

[Hardcode84]

Added, thanks.

[Hardcode84]

1.10 is out, so it can be merged, I think