New user self-registration is disabled due to spam. For an account please email bugs-admin@lists.llvm.org with your e-mail address and full name.

Bug 31751 - Compilation not completing for gnomon_engine.cpp of BlastC++
Summary: Compilation not completing for gnomon_engine.cpp of BlastC++
Status: RESOLVED FIXED
Alias: None
Product: clang
Classification: Unclassified
Component: C++ (show other bugs)
Version: trunk
Hardware: PC Linux
: P normal
Assignee: Unassigned Clang Bugs
URL:
Keywords:
Depends on:
Blocks: 31622
  Show dependency tree
 
Reported: 2017-01-25 01:16 PST by Bala Rishi
Modified: 2017-02-02 16:20 PST (History)
6 users (show)

See Also:
Fixed By Commit(s):


Attachments
Using 'clang++ -c -w gnomon_engine.cpp -O2' compilation is not completed (1.38 KB, application/octet-stream)
2017-01-25 01:16 PST, Bala Rishi
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Bala Rishi 2017-01-25 01:16:19 PST
Created attachment 17891 [details]
Using 'clang++ -c -w gnomon_engine.cpp -O2' compilation is not completed

I have attached the reduced file to reproduce the issue.

Current behavior:
	-O0/-O1: Completed build in ~2 seconds
	-O2/-O3: Building not completed even after 300 seconds

Invocation command:
        clang++ -c -w gnomon_engine.cpp -O2
Comment 1 Hans Wennborg 2017-01-26 18:22:58 PST
Is this a regression, or does it reproduce on both 3.9 as well as trunk?
Comment 2 Bala Rishi 2017-01-27 04:20:38 PST
(In reply to comment #1)
> Is this a regression, or does it reproduce on both 3.9 as well as trunk?

Yes, this is a regression. As this issue was introduced in Nov 2016, can't be reproduced on 3.9.


With the below one the issue is NOT reproduced:
Repo		SVN ID	Git Revision					Author
LLVM		286137	99d2cab4ae680d938bed8a8f1c3869a2b014f40d	Davide Italiano 
clang		286132	777d844d0f09ca78023557ac0c8318d6061c4188	Eric Liu
compiler-rt	286136	806c7f2f5200a5a54e18e69a01f2060fa99958a2	Kuba Brecka


With the below one the issue is REPRDUCED:
Repo		SVN ID	Git Revision					Author
llvm		286423	9c5e4bac4a745f6a621dcf2803fa89f6bbe51862	Sanjay Patel
clang	 	286421	3b61b92e0fdf5093a247270ef1265c85697de3f9	Argyrios Kyrtzidis
compiler-rt	286395	f2de077dc7d89466b630c308ed8eaf6bb4915210	Reid Kleckner
Comment 3 Sanjay Patel 2017-01-27 09:17:10 PST
(In reply to comment #2)
> llvm		286423	9c5e4bac4a745f6a621dcf2803fa89f6bbe51862	Sanjay Patel

Based on that, I thought this has going to be a shuffle+extract+insert problem, but this looks like a min/max infinite loop. Reducing an IR test case for InstCombine now.
Comment 4 Sanjay Patel 2017-01-27 09:43:14 PST
That's all it takes:

define double @PR31751(i32 %x) {
  %cmp = icmp slt i32 %x, 0
  %sel = select i1 %cmp, i32 2147483647, i32 %x
  %conv = sitofp i32 %sel to double
  ret double %conv
}

$ ./opt -instcombine mininfloop.ll -S

--------------------------------------------------------------------------------

The commit where this actually became a problem should be:
https://reviews.llvm.org/rL286318

There must be a sitofp transform that is interacting badly with the fact that we now recognize the icmp+select as a weird form of UMIN.
Comment 5 Sanjay Patel 2017-01-27 12:44:28 PST
We have multiple personality disorder. Canonical form changes because we treat min/max as special cases in some places and we're not consistent about what constitutes min/max. Ie, we have m_UMax and friends, matchSelectPattern, or roll your own:

  if (I.hasOneUse())
    if (SelectInst *SI = dyn_cast<SelectInst>(*I.user_begin()))
      if ((SI->getOperand(1) == Op0 && SI->getOperand(2) == Op1) ||
          (SI->getOperand(2) == Op0 && SI->getOperand(1) == Op1))
        return nullptr;


I think the fix for this bug will involve moving this misplaced transform in visitICmpInst:

      // (x >u 2147483647) -> (x <s 0)  -> true if sign bit set

...but we really need to clean up min/max in a bigger way or there will be more problems.
Comment 6 Hans Wennborg 2017-01-27 16:48:20 PST
Sanjay, did you intentionally remove this as a blocker from PR31622? If it's indeed a regression from 3.9, I'd be nice to get some kind of fix before the release.
Comment 7 Sanjay Patel 2017-01-27 17:06:50 PST
(In reply to comment #6)
> Sanjay, did you intentionally remove this as a blocker from PR31622? If it's
> indeed a regression from 3.9, I'd be nice to get some kind of fix before the
> release.

No - I must have accidentally removed the blocker when I added the last comment. Let me restore that. Also, I should have a patch posted shortly.
Comment 8 Sanjay Patel 2017-01-27 18:40:16 PST
Patch checked into trunk here:
https://reviews.llvm.org/rL293345
Comment 9 Bala Rishi 2017-01-28 12:30:51 PST
(In reply to comment #8)
> Patch checked into trunk here:
> https://reviews.llvm.org/rL293345

Thank you for the quick turnaround.
Comment 10 Hans Wennborg 2017-02-02 16:20:40 PST
(In reply to comment #8)
> Patch checked into trunk here:
> https://reviews.llvm.org/rL293345

Merged to 4.0 in r293947.