llvm dev - May 2020 - RFC: [GlobalISel] propagating int/float type information

I don’t think bfloat should be handled this way. What Amara is suggesting is an
optimization, i.e., if we drop the information we are still correct.
With bfloat, if we do an operation on float16 instead of bfloat16 this is a
correctness problem.

So that means that either we need to have new opcodes for bfloat or we need to
carry around the floating point type in MIR. I think it would be more manageable
to have the floating point type long term.
That said, it also depends on what we decide to do at the IR level. For
instance, if bfloat support in the IR is limited to intrinsics, we wouldn’t need
to go down that road.
> On May 5, 2020, at 4:19 AM, Ties Stuij via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> I don't know too much about GlobalIsel, but I'm working on adding a
new bfloat IR/MVT type (16-bit float type) to LLVM, and on one of the patches
Amara raised the issue what we would to to disambiguate between a half and a
bfloat for GlobalIsel.
> 
> Just wanted to highlight that BFloat might be another use-case for this.
> 
> Cheers,
> /Ties
> 
> ________________________________________
> From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of
Amara Emerson via llvm-dev <llvm-dev at lists.llvm.org>
> Sent: 01 May 2020 18:12
> To: LLVM Developers' List
> Cc: Matt Arsenault
> Subject: [llvm-dev] RFC: [GlobalISel] propagating int/float type
information
> 
> Hi,
> 
> GlobalISel currently drops all type information relating to the integer/FP
distinction during the IR translation pass, as the LLT types only represent
whether a value is a scalar/vector/pointer and it’s size/shape. To compensate,
later passes use the FP operations on those values to guess what kind of value
is being stored within that virtual register.
> 
> This means that i32/float loads get translated into the same thing, and
only when that value is used, say by an fadd, then will we know that it was an
FP value. The regbankselect pass on AArch64 currently tries to walk uses/defs in
order to guess what kind fo regbank to assign to vregs. This however doesn’t
work all the time, and most commonly, it doesn’t work when a load of an FP value
is used in a loop. In that case, the FP users are obscured by PHIs which make it
difficult (although not strictly impossible) to guess what regbank to assign.
This has drastic consequences for performance on FP workloads.
> 
> But this isn’t the first time we’ve had this kind of issue, and it probably
won’t be the last [1].  propose that we have some form of type hint propagation
done at the IRTranslator stage in order to make this whole situation easier (and
faster in compile-time).
> 
> Option 1) We use some form of metadata on the MIR instructions like G_LOADs
to signify that the vreg defined likely has an FP IR type. IIUC the current
Metadata MachineOperand type is only intended for debug info. This approach is
probably the cheapest in compile time/complexity and is the least invasive, but
we’d need to find somewhere in MachineInstr to store this extra information.
> 
> Option 2) Store the type hints in an analysis. In its simplest form at
translation time we could keep a set of all the vregs that we know have FP types
and then try to maintain that as new vregs are created to replace those
throughout the pipeline. Keeping it updated might turn out to be expensive
during passes like the legalizer.
> 
> Any thoughts?
> 
> Cheers,
> Amara
> 
> [1] Currently we have a workaround for the specific case in
https://reviews.llvm.org/D79207, but as Matt correctly points out, this isn’t
viable in the long term because using the IR value type from the
MachineMemOperand won’t work when opaque pointers finally land.
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

> On May 5, 2020, at 9:59 AM, Quentin Colombet <qcolombet at apple.com>
wrote:
> 
> I don’t think bfloat should be handled this way. What Amara is suggesting
is an optimization, i.e., if we drop the information we are still correct.
> With bfloat, if we do an operation on float16 instead of bfloat16 this is a
correctness problem.
> 
> So that means that either we need to have new opcodes for bfloat or we need
to carry around the floating point type in MIR. I think it would be more
manageable to have the floating point type long term.
> That said, it also depends on what we decide to do at the IR level. For
instance, if bfloat support in the IR is limited to intrinsics, we wouldn’t need
to go down that road.
> 
>> On May 5, 2020, at 4:19 AM, Ties Stuij via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
>> 
>> I don't know too much about GlobalIsel, but I'm working on
adding a new bfloat IR/MVT type (16-bit float type) to LLVM, and on one of the
patches Amara raised the issue what we would to to disambiguate between a half
and a bfloat for GlobalIsel.
I don’t think it was me who said that but it’s a good point.

I see no good reason not to re-use the existing IR instructions for computation
on BFloat, given that we do so for other uncommon FP formats as well. Which I
think in turn leaves us with little choice but to mirror the IR here (adding
separate opcodes for this case just seems wrong).
>> 
>> Just wanted to highlight that BFloat might be another use-case for
this.
>> 
>> Cheers,
>> /Ties
>> 
>> ________________________________________
>> From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of
Amara Emerson via llvm-dev <llvm-dev at lists.llvm.org>
>> Sent: 01 May 2020 18:12
>> To: LLVM Developers' List
>> Cc: Matt Arsenault
>> Subject: [llvm-dev] RFC: [GlobalISel] propagating int/float type
information
>> 
>> Hi,
>> 
>> GlobalISel currently drops all type information relating to the
integer/FP distinction during the IR translation pass, as the LLT types only
represent whether a value is a scalar/vector/pointer and it’s size/shape. To
compensate, later passes use the FP operations on those values to guess what
kind of value is being stored within that virtual register.
>> 
>> This means that i32/float loads get translated into the same thing, and
only when that value is used, say by an fadd, then will we know that it was an
FP value. The regbankselect pass on AArch64 currently tries to walk uses/defs in
order to guess what kind fo regbank to assign to vregs. This however doesn’t
work all the time, and most commonly, it doesn’t work when a load of an FP value
is used in a loop. In that case, the FP users are obscured by PHIs which make it
difficult (although not strictly impossible) to guess what regbank to assign.
This has drastic consequences for performance on FP workloads.
>> 
>> But this isn’t the first time we’ve had this kind of issue, and it
probably won’t be the last [1].  propose that we have some form of type hint
propagation done at the IRTranslator stage in order to make this whole situation
easier (and faster in compile-time).
>> 
>> Option 1) We use some form of metadata on the MIR instructions like
G_LOADs to signify that the vreg defined likely has an FP IR type. IIUC the
current Metadata MachineOperand type is only intended for debug info. This
approach is probably the cheapest in compile time/complexity and is the least
invasive, but we’d need to find somewhere in MachineInstr to store this extra
information.
>> 
>> Option 2) Store the type hints in an analysis. In its simplest form at
translation time we could keep a set of all the vregs that we know have FP types
and then try to maintain that as new vregs are created to replace those
throughout the pipeline. Keeping it updated might turn out to be expensive
during passes like the legalizer.
>> 
>> Any thoughts?
>> 
>> Cheers,
>> Amara
>> 
>> [1] Currently we have a workaround for the specific case in
https://reviews.llvm.org/D79207, but as Matt correctly points out, this isn’t
viable in the long term because using the IR value type from the
MachineMemOperand won’t work when opaque pointers finally land.
>> 
>> 
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

Quentin: Thanks for the info. I was under the impression that the LLVM community
at large would prefer to extend the IR type to a bfloat MVT type. I've made
a number of patches to implement this up to a point for AArch64. I can post
those on Phab and start a thread to sample opinions.

Amara: Ah yes, I see now it was Matt Arsenault who made the comment. And I see
he happens to be CC'ed on this issue. your Phab abbreviations are quite
similar.

________________________________________
From: Quentin Colombet <qcolombet at apple.com>
Sent: 05 May 2020 17:59
To: Ties Stuij
Cc: LLVM Developers' List; Amara Emerson; Matt Arsenault
Subject: Re: [llvm-dev] RFC: [GlobalISel] propagating int/float type information

I don’t think bfloat should be handled this way. What Amara is suggesting is an
optimization, i.e., if we drop the information we are still correct.
With bfloat, if we do an operation on float16 instead of bfloat16 this is a
correctness problem.

So that means that either we need to have new opcodes for bfloat or we need to
carry around the floating point type in MIR. I think it would be more manageable
to have the floating point type long term.
That said, it also depends on what we decide to do at the IR level. For
instance, if bfloat support in the IR is limited to intrinsics, we wouldn’t need
to go down that road.
> On May 5, 2020, at 4:19 AM, Ties Stuij via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
>
> I don't know too much about GlobalIsel, but I'm working on adding a
new bfloat IR/MVT type (16-bit float type) to LLVM, and on one of the patches
Amara raised the issue what we would to to disambiguate between a half and a
bfloat for GlobalIsel.
>
> Just wanted to highlight that BFloat might be another use-case for this.
>
> Cheers,
> /Ties
>
> ________________________________________
> From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of
Amara Emerson via llvm-dev <llvm-dev at lists.llvm.org>
> Sent: 01 May 2020 18:12
> To: LLVM Developers' List
> Cc: Matt Arsenault
> Subject: [llvm-dev] RFC: [GlobalISel] propagating int/float type
information
>
> Hi,
>
> GlobalISel currently drops all type information relating to the integer/FP
distinction during the IR translation pass, as the LLT types only represent
whether a value is a scalar/vector/pointer and it’s size/shape. To compensate,
later passes use the FP operations on those values to guess what kind of value
is being stored within that virtual register.
>
> This means that i32/float loads get translated into the same thing, and
only when that value is used, say by an fadd, then will we know that it was an
FP value. The regbankselect pass on AArch64 currently tries to walk uses/defs in
order to guess what kind fo regbank to assign to vregs. This however doesn’t
work all the time, and most commonly, it doesn’t work when a load of an FP value
is used in a loop. In that case, the FP users are obscured by PHIs which make it
difficult (although not strictly impossible) to guess what regbank to assign.
This has drastic consequences for performance on FP workloads.
>
> But this isn’t the first time we’ve had this kind of issue, and it probably
won’t be the last [1].  propose that we have some form of type hint propagation
done at the IRTranslator stage in order to make this whole situation easier (and
faster in compile-time).
>
> Option 1) We use some form of metadata on the MIR instructions like G_LOADs
to signify that the vreg defined likely has an FP IR type. IIUC the current
Metadata MachineOperand type is only intended for debug info. This approach is
probably the cheapest in compile time/complexity and is the least invasive, but
we’d need to find somewhere in MachineInstr to store this extra information.
>
> Option 2) Store the type hints in an analysis. In its simplest form at
translation time we could keep a set of all the vregs that we know have FP types
and then try to maintain that as new vregs are created to replace those
throughout the pipeline. Keeping it updated might turn out to be expensive
during passes like the legalizer.
>
> Any thoughts?
>
> Cheers,
> Amara
>
> [1] Currently we have a workaround for the specific case in
https://reviews.llvm.org/D79207, but as Matt correctly points out, this isn’t
viable in the long term because using the IR value type from the
MachineMemOperand won’t work when opaque pointers finally land.
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

> On May 5, 2020, at 11:14 AM, Amara Emerson <aemerson at apple.com>
wrote:
> 
> 
> 
>> On May 5, 2020, at 9:59 AM, Quentin Colombet <qcolombet at
apple.com> wrote:
>> 
>> I don’t think bfloat should be handled this way. What Amara is
suggesting is an optimization, i.e., if we drop the information we are still
correct.
>> With bfloat, if we do an operation on float16 instead of bfloat16 this
is a correctness problem.
>> 
>> So that means that either we need to have new opcodes for bfloat or we
need to carry around the floating point type in MIR. I think it would be more
manageable to have the floating point type long term.
>> That said, it also depends on what we decide to do at the IR level. For
instance, if bfloat support in the IR is limited to intrinsics, we wouldn’t need
to go down that road.
>> 
>>> On May 5, 2020, at 4:19 AM, Ties Stuij via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
>>> 
>>> I don't know too much about GlobalIsel, but I'm working on
adding a new bfloat IR/MVT type (16-bit float type) to LLVM, and on one of the
patches Amara raised the issue what we would to to disambiguate between a half
and a bfloat for GlobalIsel.
> I don’t think it was me who said that but it’s a good point.
> 
> I see no good reason not to re-use the existing IR instructions for
computation on BFloat, given that we do so for other uncommon FP formats as
well. Which I think in turn leaves us with little choice but to mirror the IR
here (adding separate opcodes for this case just seems wrong).
Agree!
>>> 
>>> Just wanted to highlight that BFloat might be another use-case for
this.
>>> 
>>> Cheers,
>>> /Ties
>>> 
>>> ________________________________________
>>> From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf
of Amara Emerson via llvm-dev <llvm-dev at lists.llvm.org>
>>> Sent: 01 May 2020 18:12
>>> To: LLVM Developers' List
>>> Cc: Matt Arsenault
>>> Subject: [llvm-dev] RFC: [GlobalISel] propagating int/float type
information
>>> 
>>> Hi,
>>> 
>>> GlobalISel currently drops all type information relating to the
integer/FP distinction during the IR translation pass, as the LLT types only
represent whether a value is a scalar/vector/pointer and it’s size/shape. To
compensate, later passes use the FP operations on those values to guess what
kind of value is being stored within that virtual register.
>>> 
>>> This means that i32/float loads get translated into the same thing,
and only when that value is used, say by an fadd, then will we know that it was
an FP value. The regbankselect pass on AArch64 currently tries to walk uses/defs
in order to guess what kind fo regbank to assign to vregs. This however doesn’t
work all the time, and most commonly, it doesn’t work when a load of an FP value
is used in a loop. In that case, the FP users are obscured by PHIs which make it
difficult (although not strictly impossible) to guess what regbank to assign.
This has drastic consequences for performance on FP workloads.
>>> 
>>> But this isn’t the first time we’ve had this kind of issue, and it
probably won’t be the last [1].  propose that we have some form of type hint
propagation done at the IRTranslator stage in order to make this whole situation
easier (and faster in compile-time).
>>> 
>>> Option 1) We use some form of metadata on the MIR instructions like
G_LOADs to signify that the vreg defined likely has an FP IR type. IIUC the
current Metadata MachineOperand type is only intended for debug info. This
approach is probably the cheapest in compile time/complexity and is the least
invasive, but we’d need to find somewhere in MachineInstr to store this extra
information.
>>> 
>>> Option 2) Store the type hints in an analysis. In its simplest form
at translation time we could keep a set of all the vregs that we know have FP
types and then try to maintain that as new vregs are created to replace those
throughout the pipeline. Keeping it updated might turn out to be expensive
during passes like the legalizer.
>>> 
>>> Any thoughts?
>>> 
>>> Cheers,
>>> Amara
>>> 
>>> [1] Currently we have a workaround for the specific case in
https://reviews.llvm.org/D79207, but as Matt correctly points out, this isn’t
viable in the long term because using the IR value type from the
MachineMemOperand won’t work when opaque pointers finally land.
>>> 
>>> 
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200505/a18ae3f7/attachment.html>

> On May 5, 2020, at 2:45 PM, Ties Stuij <Ties.Stuij at arm.com> wrote:
> 
> Quentin: Thanks for the info. I was under the impression that the LLVM
community at large would prefer to extend the IR type to a bfloat MVT type.
I've made a number of patches to implement this up to a point for AArch64. I
can post those on Phab and start a thread to sample opinions.
Sounds good to me!

I also think having bfloat (all floating point types actually)  in GISel (i.e.,
LLT) is also the right long term plan. Maybe something like a pair of two
numbers for the number of bits in the mantissa and the number of bits in the
exponent would be enough to represent all of them.
> 
> Amara: Ah yes, I see now it was Matt Arsenault who made the comment. And I
see he happens to be CC'ed on this issue. your Phab abbreviations are quite
similar.
> 
> ________________________________________
> From: Quentin Colombet <qcolombet at apple.com>
> Sent: 05 May 2020 17:59
> To: Ties Stuij
> Cc: LLVM Developers' List; Amara Emerson; Matt Arsenault
> Subject: Re: [llvm-dev] RFC: [GlobalISel] propagating int/float type
information
> 
> I don’t think bfloat should be handled this way. What Amara is suggesting
is an optimization, i.e., if we drop the information we are still correct.
> With bfloat, if we do an operation on float16 instead of bfloat16 this is a
correctness problem.
> 
> So that means that either we need to have new opcodes for bfloat or we need
to carry around the floating point type in MIR. I think it would be more
manageable to have the floating point type long term.
> That said, it also depends on what we decide to do at the IR level. For
instance, if bfloat support in the IR is limited to intrinsics, we wouldn’t need
to go down that road.
> 
>> On May 5, 2020, at 4:19 AM, Ties Stuij via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
>> 
>> I don't know too much about GlobalIsel, but I'm working on
adding a new bfloat IR/MVT type (16-bit float type) to LLVM, and on one of the
patches Amara raised the issue what we would to to disambiguate between a half
and a bfloat for GlobalIsel.
>> 
>> Just wanted to highlight that BFloat might be another use-case for
this.
>> 
>> Cheers,
>> /Ties
>> 
>> ________________________________________
>> From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of
Amara Emerson via llvm-dev <llvm-dev at lists.llvm.org>
>> Sent: 01 May 2020 18:12
>> To: LLVM Developers' List
>> Cc: Matt Arsenault
>> Subject: [llvm-dev] RFC: [GlobalISel] propagating int/float type
information
>> 
>> Hi,
>> 
>> GlobalISel currently drops all type information relating to the
integer/FP distinction during the IR translation pass, as the LLT types only
represent whether a value is a scalar/vector/pointer and it’s size/shape. To
compensate, later passes use the FP operations on those values to guess what
kind of value is being stored within that virtual register.
>> 
>> This means that i32/float loads get translated into the same thing, and
only when that value is used, say by an fadd, then will we know that it was an
FP value. The regbankselect pass on AArch64 currently tries to walk uses/defs in
order to guess what kind fo regbank to assign to vregs. This however doesn’t
work all the time, and most commonly, it doesn’t work when a load of an FP value
is used in a loop. In that case, the FP users are obscured by PHIs which make it
difficult (although not strictly impossible) to guess what regbank to assign.
This has drastic consequences for performance on FP workloads.
>> 
>> But this isn’t the first time we’ve had this kind of issue, and it
probably won’t be the last [1].  propose that we have some form of type hint
propagation done at the IRTranslator stage in order to make this whole situation
easier (and faster in compile-time).
>> 
>> Option 1) We use some form of metadata on the MIR instructions like
G_LOADs to signify that the vreg defined likely has an FP IR type. IIUC the
current Metadata MachineOperand type is only intended for debug info. This
approach is probably the cheapest in compile time/complexity and is the least
invasive, but we’d need to find somewhere in MachineInstr to store this extra
information.
>> 
>> Option 2) Store the type hints in an analysis. In its simplest form at
translation time we could keep a set of all the vregs that we know have FP types
and then try to maintain that as new vregs are created to replace those
throughout the pipeline. Keeping it updated might turn out to be expensive
during passes like the legalizer.
>> 
>> Any thoughts?
>> 
>> Cheers,
>> Amara
>> 
>> [1] Currently we have a workaround for the specific case in
https://reviews.llvm.org/D79207, but as Matt correctly points out, this isn’t
viable in the long term because using the IR value type from the
MachineMemOperand won’t work when opaque pointers finally land.
>> 
>> 
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

> On May 5, 2020, at 14:14, Amara Emerson <aemerson at apple.com>
wrote:
> 
> I don’t think it was me who said that but it’s a good point.
> 
> I see no good reason not to re-use the existing IR instructions for
computation on BFloat, given that we do so for other uncommon FP formats as
well. Which I think in turn leaves us with little choice but to mirror the IR
here (adding separate opcodes for this case just seems wrong).
I thought about this a bit and I think adding separate LLTs is probably the
right approach. We were ignoring the existence of ppcf128 anyway, so bfloat16
doesn’t really introduce a new issue. However, I do want to deviate from the IR
and SelectionDAG’s treatment of integer vs. FP operations to preserve the
current property GlobalISel has where integer operations are allowed to freely
operate directly on FP values. As long as I’m not required to insert bitcasts
to/from integer LLTs just to operate on the bits of an FP value, I’m OK with it.
We would only consider these different types in floating point contexts, and
they would implicitly behave as the equivalent sized integers elsewhere. The
current intermediate bitcasts needed in various FP legalization code are quite
annoying, and they’ve always been an obstacle to some combines.

-Matt
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200523/cd6ca3a7/attachment-0001.html>