[META][GVN] NewGVN Integration

[RKSimon]

Bugzilla Link	30995
Version	trunk
OS	All
Depends On	#30119 #30859 #30871 #30961 #31140 #31215 #31793 #35683 #36469 #29898 #30816 #30820 #30828 #30831 #30839 #30847 #30849 #30860 #30902 #30921 #30942 #31030 #31034 #31046 #31097 #31106 #31216 #31696 #31750 #32512 #32714 #32747 #34422 #34537 #34931 #35149 #35187 #36888 #36889 #38404 #38405 #38627 #41767
CC	@alinas,@andrewrk,@Bigcheese,@Theodor,@filcab,@fhahn,@hfinkel,@LebedevRI,@slacka,@nunoplopes,@JonPsson,@sanjoy,@stbergmann

Extended Description

Meta ticket covering the addition of the NewGVN pass.

WIP Patch: https://reviews.llvm.org/D26224

[llvmbot]

The initial support was merged upstream. I'll keep this open for a while as an umbrella for all the NewGVN issues.

[JonPsson]

FAIL: "The instructions for these memory operations should have been in the same congruence class"

You are probably already aware of this, but the test-suite fails with '-mllvm -enable-newgvn'. I am posting this here for other users sake who might also follow Davide's suggestion and run the test-suite on some target.

I ran the test-suite on SystemZ with NewGVN, and found then this crash. I then checked on X86 to also find the same fail there.

test-suite/MultiSource/Benchmarks/mafft/io.c:

clang-4.0: /home/jonas/llvm/llvm-dev/lib/Transforms/Scalar/NewGVN.cpp:1427: void NewGVN::verifyMemoryCongruency(): Assertion `ValueToClass.lookup(FirstMUD->getMemoryInst()) == ValueToClass.lookup(SecondMUD->getMemoryInst()) && "The instructions for these memory operations should have been in " "the same congruence class"' failed.

I used llvm/trunk@290872, cfe/trunk@290887 and test-suite/trunk@290894.

Options: -O3 -mllvm -enable-newgvn

/Jonas

[llvmbot]

Thanks Jonas, going to reduce/debug this soon =)

[llvmbot]

Jonas, do you mind to try ToT? Danny fixed all the remaining unknown issues.

[llvmbot]

Jonas, do you mind to try ToT? Danny fixed all the remaining unknown issues.

*known, even.

[JonPsson]

Sorry, but I haven't even opened the NewGVN.cpp file. My idea was to contribute by running the test-suite on SystemZ, which I did. I was expecting the author(s) of the code would then take the time to fix any issues.

[JonPsson]

(and yes, the crash I found is still there as far as I can see)

[llvmbot]

Sorry, but I haven't even opened the NewGVN.cpp file. My idea was to
contribute by running the test-suite on SystemZ, which I did. I was
expecting the author(s) of the code would then take the time to fix any
issues.

I did =) https://llvm.org/bugs/show_bug.cgi?id=31573

[JonPsson]

Ah, great :-)

I tried to apply the attached patch, but it did not apply cleanly anymore, at least not in my repo with latest changes.

Let me know when the test-suite passes on X86, and I will gladly run it on SystemZ.

/Jonas

[llvmbot]

Ah, great :-)

I tried to apply the attached patch, but it did not apply cleanly anymore,
at least not in my repo with latest changes.

Let me know when the test-suite passes on X86, and I will gladly run it on
SystemZ.

/Jonas

LLVM passes test-suite on X86 now, so I think we're in a good shape to try on other platforms if you want to take a look.

[llvmbot]

Ah, great :-)

I tried to apply the attached patch, but it did not apply cleanly anymore,
at least not in my repo with latest changes.

Let me know when the test-suite passes on X86, and I will gladly run it on
SystemZ.

/Jonas

LLVM passes test-suite on X86 now, so I think we're in a good shape to try
on other platforms if you want to take a look.

damn, s/LLVM/LLVM with NewGVN/

[JonPsson]

reduced test case for opt with newgvn crash

[JonPsson]

Huh, I got still another fail today on SystemZ with
llvm/trunk@292353
cfe/trunk@292343
test-suite/trunk@292129

NewGVN.cpp:1084: void NewGVN::moveValueToNewCongruenceClass(llvm::Instruction*, CongruenceClass*, CongruenceClass*): Assertion `(!isa(NewClass->RepLeader) || !NewClass->RepLeader || I == NewClass->RepLeader || !DT->properlyDominates( I->getParent(), cast(NewClass->RepLeader)->getParent())) && "New class for instruction should not be dominated by instruction"' failed.

/bin/opt -O3 -enable-newgvn -S ./newgvn_fail.ll

I postedthe reduced test-case above.

[llvmbot]

Huh, I got still another fail today on SystemZ with
llvm/trunk@292353
cfe/trunk@292343
test-suite/trunk@292129

NewGVN.cpp:1084: void
NewGVN::moveValueToNewCongruenceClass(llvm::Instruction*, CongruenceClass*,
CongruenceClass*): Assertion `(!isa(NewClass->RepLeader) ||
!NewClass->RepLeader || I == NewClass->RepLeader || !DT->properlyDominates(
I->getParent(), cast(NewClass->RepLeader)->getParent())) &&
"New class for instruction should not be dominated by instruction"' failed.

/bin/opt -O3 -enable-newgvn -S ./newgvn_fail.ll

I postedthe reduced test-case above.

Thank you very much. I'll reduce/investigate this. For the future, do you mind to open a new issue and link it here? It would make our tracking much easier =)

[llvmbot]

mentioned in issue #30816