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

[AArch64][GlobalISel] Add lowering for constant BIT/BIF/BSP #65897

Merged
merged 1 commit into from
Sep 12, 2023

Conversation

davemgreen
Copy link
Collaborator

I have been making my way through GlobalISel vector fp, making sure all the basic types are currently working. In looking at fcopysign I managed to rewrite it in a way that we just widen to vectors, as opposed to relying on custom lowering. That misses handling vector constants (which Mark here is currently looking at) and transforming or/and into bit.

The non-constant bit/bif/bsp already work through tablegen patters, this patch handles the constant case, mirroring the basic support for or(and(X, C), and(Y, ~C)) from ISel tryCombineToBSL. BSP gets expanded to either BIT, BIF or BSL depending on the best register allocation. G_BIT can be replaced with G_BSP as a more general alternative.

@@ -216,7 +216,7 @@ def G_PREFETCH : AArch64GenericInstruction {
}

// Generic bitwise insert if true.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment needs updating.

@@ -216,7 +216,7 @@ def G_PREFETCH : AArch64GenericInstruction {
}

// Generic bitwise insert if true.
def G_BIT : AArch64GenericInstruction {
def G_BSP : AArch64GenericInstruction {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this opcode needs a bit of documentation since it's not a real AArch64 instruction.

Comment on lines 347 to 356
MachineInstr *LHS =
getOpcodeDef(TargetOpcode::G_AND, MI.getOperand(1).getReg(), MRI);
MachineInstr *RHS =
getOpcodeDef(TargetOpcode::G_AND, MI.getOperand(2).getReg(), MRI);
if (!LHS || !RHS)
return false;

auto *BV1 = getOpcodeDef<GBuildVector>(LHS->getOperand(2).getReg(), MRI);
auto *BV2 = getOpcodeDef<GBuildVector>(RHS->getOperand(2).getReg(), MRI);
if (!BV1 || !BV2)
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least some of this could be done with mi_match()? Not a big deal either way.

Comment on lines 369 to 371
std::get<0>(MatchInfo) = LHS->getOperand(1).getReg();
std::get<1>(MatchInfo) = RHS->getOperand(1).getReg();
std::get<2>(MatchInfo) = RHS->getOperand(2).getReg();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can just do MatchInfo = {v1,v2,v3};

Comment on lines 359 to 361
for (int k = 0; k < DstTy.getNumElements(); k++) {
auto ValAndVReg1 = getIConstantVRegValWithLookThrough(
BV1->getOperand(k + 1).getReg(), MRI);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use GBuildVector::getSourceReg(k).

auto ValAndVReg2 = getIConstantVRegValWithLookThrough(
BV2->getOperand(k + 1).getReg(), MRI);
if (!ValAndVReg1 || !ValAndVReg2 ||
ValAndVReg1->Value != ~ValAndVReg2->Value)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer *ValAndReg1 for ValAndReg2 precedence is not obvious.

if (!BV1 || !BV2)
return false;

for (int k = 0; k < DstTy.getNumElements(); k++) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for (int k = 0, int NumElements = DstTy.getNumElements(); k < NumElements; ++k)

Copy link
Member

@tschuett tschuett Sep 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the far future a scalable vector might sneak in, then you cannot walk over its members.

The non-constant versions already work through tablegen patters, this mirrors
the basic support for or(and(X, C), and(Y, ~C)) from ISel tryCombineToBSL. BSP
gets expanded to either BIT, BIF or BSL depending on the best register
allocation. G_BIT can be replaced with G_BSP as a more general alternative.
Copy link
Contributor

@aemerson aemerson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@davemgreen davemgreen merged commit cb5bad2 into llvm:main Sep 12, 2023
2 checks passed
@davemgreen davemgreen deleted the gh-gi-bsp branch September 12, 2023 09:13
@jeanPerier
Copy link
Contributor

Hi @davemgreen, I am seeing flang/clang failures in the llvm-test-suite after this patch with tests involving copy sign:

Can you check if this is related to your patch that involves copy sign on aarch64?

@davemgreen
Copy link
Collaborator Author

Hello. It sounds like it might be related. I might not have updated the existing copysign code correctly, as I was planning to remove it eventually. I will revert and take a look.

davemgreen added a commit that referenced this pull request Sep 17, 2023
The non-constant bit/bif/bsp already work through tablegen patterns, this
patch handles the constant case, mirroring the basic support for
`or(and(X, C), and(Y, ~C))` from ISel tryCombineToBSL. BSP gets expanded
to either BIT, BIF or BSL depending on the best register allocation.
G_BIT can be replaced with G_BSP as a more general alternative.
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
The non-constant bit/bif/bsp already work through tablegen patterns, this
patch handles the constant case, mirroring the basic support for
`or(and(X, C), and(Y, ~C))` from ISel tryCombineToBSL. BSP gets expanded
to either BIT, BIF or BSL depending on the best register allocation.
G_BIT can be replaced with G_BSP as a more general alternative.
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
The non-constant bit/bif/bsp already work through tablegen patterns, this
patch handles the constant case, mirroring the basic support for
`or(and(X, C), and(Y, ~C))` from ISel tryCombineToBSL. BSP gets expanded
to either BIT, BIF or BSL depending on the best register allocation.
G_BIT can be replaced with G_BSP as a more general alternative.
zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023
The non-constant bit/bif/bsp already work through tablegen patterns, this
patch handles the constant case, mirroring the basic support for
`or(and(X, C), and(Y, ~C))` from ISel tryCombineToBSL. BSP gets expanded
to either BIT, BIF or BSL depending on the best register allocation.
G_BIT can be replaced with G_BSP as a more general alternative.
zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023
The non-constant bit/bif/bsp already work through tablegen patterns, this
patch handles the constant case, mirroring the basic support for
`or(and(X, C), and(Y, ~C))` from ISel tryCombineToBSL. BSP gets expanded
to either BIT, BIF or BSL depending on the best register allocation.
G_BIT can be replaced with G_BSP as a more general alternative.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants