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

DAGCombiner doesn't check legality for merging stores with noimplicitfloat #39641

Open
nemanjai opened this issue Jan 11, 2019 · 4 comments
Open
Labels
bugzilla Issues migrated from bugzilla llvm:codegen llvm:crash

Comments

@nemanjai
Copy link
Member

nemanjai commented Jan 11, 2019

Bugzilla Link 40294
Version trunk
OS All

Extended Description

This can be reproduced as follows:

$ cat a.ll 
define dso_local void @test(i64* nocapture %arr1) local_unnamed_addr #0 {
entry:
  %arrayidx = getelementptr inbounds i64, i64* %arr1, i64 2
  %0 = load i64, i64* %arrayidx, align 8
  store i64 %0, i64* %arr1, align 8
  %arrayidx2 = getelementptr inbounds i64, i64* %arr1, i64 3
  %1 = load i64, i64* %arrayidx2, align 8
  %arrayidx3 = getelementptr inbounds i64, i64* %arr1, i64 1
  store i64 %1, i64* %arrayidx3, align 8
  ret void
}

attributes #0 = { noimplicitfloat }

$ llc a.ll -mtriple=powerpc64le-unknown-unknown
llc: /home/nemanjai/llvm/llvm-clean/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:970: void {anonymous}::SelectionDAGLegalize::LegalizeOp(llvm::SDNode*): Assertion `(TLI.getTypeAction(*DAG.getContext(), Op.getValueType()) == TargetLowering::TypeLegal || TLI.isTypeLegal(Op.getValueType()) || Op.getOpcode() == ISD::TargetConstant || Op.getOpcode() == ISD::Register) && "Unexpected illegal type!"' failed.

The problem is that PPC does not override TargetLowering::mergeStoresAfterLegalization() and the merging will produce an i128 which isn't legal.
It will of course produce it prior to legalization as well, but the legalizer will undo that, the problem is after legalization.

Note that without the noimplicitfloat attribute, things work because using vectors is allowed and then it will find a corresponding vector type.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
trevor-m pushed a commit to trevor-m/llvm-project that referenced this issue Apr 20, 2023
trevor-m pushed a commit to trevor-m/llvm-project that referenced this issue Apr 20, 2023
Fix llvm#39641

Recommit of r366413

Differential Revision: https://reviews.llvm.org/D63877

llvm-svn: 366588
trevor-m pushed a commit to trevor-m/llvm-project that referenced this issue Apr 20, 2023
Fix llvm#39641

Recommit of r366413

Differential Revision: https://reviews.llvm.org/D63877

llvm-svn: 366632
trevor-m pushed a commit to trevor-m/llvm-project that referenced this issue Apr 20, 2023
Fix llvm#39641

Recommit of r366413

Differential Revision: https://reviews.llvm.org/D63877

> llvm-svn: 366632

llvm-svn: 366638
@llvmbot
Copy link
Member

llvmbot commented Aug 5, 2023

@llvm/issue-subscribers-backend-powerpc

@nemanjai
Copy link
Member Author

nemanjai commented Aug 7, 2023

This isn't really specific to the PPC back end. The problem is in the legalizer.

@arsenm
Copy link
Contributor

arsenm commented Aug 7, 2023

Right but anything like this needs to be connected to a relevant backend. Nobody is going to fix an abstract issue for a combination of legal operations that isn't really used

@nemanjai
Copy link
Member Author

nemanjai commented Aug 8, 2023

Fair enough. I'll post a patch for this as soon as I get a chance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla llvm:codegen llvm:crash
Projects
None yet
Development

No branches or pull requests

5 participants