-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[X86] X86TargetLowering::computeKnownBitsForTargetNode - add X86ISD::VPMADD52L\H handling - again #159230
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
…ufficient leading zero/sign bits-1
…ufficient leading zero/sign bits -2
…ufficient leading zero/sign bits -3
…ufficient leading zero/sign bits-4
…ufficient leading zero/sign bits-5
…ufficient leading zero/sign bits-6
…ufficient leading zero/sign bits-7
…ufficient leading zero/sign bits-8
…ufficient leading zero/sign bits-9
…ent leading zero/sign bits-10
…ufficient leading zero/sign bits-11
…VPMADD52L\H handling-1
RKSimon
left a comment
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.
a few minors
| @@ -0,0 +1,138 @@ | |||
| ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5 | |||
| ; RUN: llc < %s -mtriple=x86_64-- -mattr=+avx512ifma,+avx512vl | FileCheck %s --check-prefixes=AVX512VL | |||
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.
we should add avxifma support, even if it means we drop the 512-bit test coverage
| %r = call <2 x i64> @llvm.x86.avx512.vpmadd52h.uq.128( | ||
| <2 x i64> <i64 1, i64 1>, ; acc | ||
| <2 x i64> %mx, ; x (masked to 25-bit) | ||
| <2 x i64> %my) ; y (masked to 25-bit) |
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.
these per-operand comment really aren't necessary - a short single line comment above the define along witha descriptive function name is all that is necessary
| ; AVX512VL-NEXT: # xmm0 = mem[0,0] | ||
| ; AVX512VL-NEXT: retq | ||
| %mx = and <2 x i64> %x, <i64 33554431, i64 33554431> | ||
| %my = and <2 x i64> %y, <i64 33554431, i64 33554431> |
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.
Try to use splat for uniform constant for breveity and remove the "25-bit/26-bit" comments and put them in the IR - nobody is ever going to go looking far for a description of a constant
%mx = and <2 x i64> %x, splat (i64 33554431) ; (1<<25)-1
%my = and <2 x i64> %y, splat (i64 33554431) ; (1<<25)-1
RKSimon
left a comment
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 - cheers
FIX #155386
My LLVM version was too old, so I updated to a newer one.
@RKSimon