-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[X86][GlobalIsel] Support G_IS_FPCLASS #162232
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
Conversation
Current generic lowering differs from SDAG in two ways.
|
Missing Optimization issue #162248 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
; RUN: llc < %s -mtriple=i686-linux -global-isel -global-isel-abort=1 | FileCheck %s -check-prefixes=X86,X86-GISEL | ||
; RUN: llc < %s -mtriple=x86_64-linux -global-isel -global-isel-abort=1 | FileCheck %s -check-prefixes=X64,X64-GISEL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe drop X86
and X64
from GlobalISel? Or introduce a common prefix for SDAG and FASTISEL. FASTISEL duplicates SDAG significantly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing X86 check label from globalisel run is conflict, X64 removal helps to reduce lines by ~70 lines.
Common prefix for DAG and FASTISEL was not much useful, reported message of unused prefix at bottom.
Change updated here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably should have vector tests but those can be added later
If no objections, can we merge? |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/187/builds/12521 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/17095 Here is the relevant piece of the build log for the reference
|
@mahesh-attarde please can you investigate the EXPENSIVE_CHECKS build failures? |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/186/builds/13164 Here is the relevant piece of the build log for the reference
|
I am working on it. |
Some of dependency opcodes for G_IS_FPCLASS are now supported. This patch adds lowering for G_IS_FPCLASS. Test is updated for GISEL Run separately [Test PR] llvm#160741.
Failure reported here is related to G_CONSTANT i8 0. GIR. GISEL LOG
DAG LOG
I tried few approaches to deal with same, those ended up with regressions on DAG. I this takes up more time, I will submit disabling for i686 RUN and continue fixing it. |
Basically the problem is that for this code SDAG can use |
Regrettably, I think its time to revert this and get EXPENSIVE_CHECKS green again. |
Reverts #162232 due to failures on EXPENSIVE_CHECKS enabled targets
Some of dependency opcodes for G_IS_FPCLASS are now supported. This patch adds lowering for G_IS_FPCLASS.
Test is updated for GISEL Run separately Test PR.