Add support for bfloat in VInsert PreLegalizerCombiner #243
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
abhinay-anubola
requested review from
abnikant,
andcarminati,
gbossu,
khallouh,
konstantinschwarz,
martien-de-jong,
SagarMaheshwari99 and
stephenneuendorffer
as code owners
abhinay-anubola
force-pushed
the
sanubola.support.bf512.VInsertCombiner
branch
from
d6afe1c to
85d6911
Compare
llvm/test/CodeGen/AIE/aie2/GlobalISel/combine-set-extract-prelegalizer.mir
Outdated
Show resolved
Hide resolved
gbossu
reviewed
Nov 25, 2024
abhinay-anubola
force-pushed
the
sanubola.support.bf512.VInsertCombiner
branch
3 times, most recently
from
f10042d to
39c7708
Compare
| @@ -527,9 +527,9 @@ INTRINSIC(v32bfloat16) insert(v32bfloat16 a, int idx, v16bfloat16 b) { | |||
| // Set 256-bit portion of 512-bit register | |||
| INTRINSIC(v32bfloat16) set_v32bfloat16(int idx, v16bfloat16 b) { | |||
| if (idx == 0) | |||
| return __builtin_aiev2_set_bf512_bf256(b, 0); | |||
| return __builtin_aiev2_set_I512_I256(b, 0); | |||
There was a problem hiding this comment.
This looks curious, all values different from 0 can be used as index 1. I was wondering why not simply:
return __builtin_aiev2_set_I512_I256(b, idx == 0 ? 0 : 1);
There was a problem hiding this comment.
I think those headers were partly auto-generated, and for some intrinsics we have more than two different values. So I'd assume it's easier to write a generic generator if sticking to if/else.
abhinay-anubola
force-pushed
the
sanubola.support.bf512.VInsertCombiner
branch
from
39c7708 to
d234b71
Compare
gbossu
reviewed
Dec 23, 2024
| MachineInstr * | ||
| llvm::getDefIgnoringCopiesAndBitcasts(Register Reg, | ||
| llvm::getDefIgnoringCopiesAndBitcasts(Register Reg, bool AllowMultiUse, | ||
| const MachineRegisterInfo &MRI) { | ||
|
|
There was a problem hiding this comment.
You seem to never use AllowMultiUse=true. Do you have future plans outside of this PR?
There was a problem hiding this comment.
Oh okay there is one use in matchExtractConcat. I would be curious to see if allowing multiple uses in other combiners would help as well?
gbossu
reviewed
Dec 30, 2024
gbossu
reviewed
Dec 30, 2024
| @@ -191,6 +191,7 @@ unsigned getVInsertScalarSize(unsigned IntrinsicID) { | |||
| case Intrinsic::aie2_vinsert8_I512: | |||
| return 8; | |||
| case Intrinsic::aie2_vinsert16_I512: | |||
| case Intrinsic::aie2_vinsert16_bf512: | |||
There was a problem hiding this comment.
I think we should also get rid of that intrinsic at some point
abhinay-anubola
force-pushed
the
sanubola.support.bf512.VInsertCombiner
branch
2 times, most recently
from
534837e to
9d57d0f
Compare
abhinay-anubola
force-pushed
the
sanubola.support.bf512.VInsertCombiner
branch
from
9d57d0f to
2c573c6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add support for bfloat in SetExtract PreLegalizerCombiner
Add support for bfloat in VInsert PreLegalizerCombiner