Previously in addTypeForNeon, we would set the operations for bfloat vectors
like other generic types. But as bfloat is a storage-only type a number of
operations shouldn't be set. This patch fixes that.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
Quick question - what is the expected behaviour? Do we just never expect to see an bf16 add, and if we do it's a fatal error? Or is some form of automatic promotion expected to happen?
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp | ||
---|---|---|
1021 | These are integer, so I wouldn't expect then to cause much trouble. | |
1045 | Why not leave these as expand? |
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp | ||
---|---|---|
1045 | And they are integers as well, no? Perhaps the correct tests we want here is !VT.isFloatingPoint() or alike? |
Yes, I don't think we should see explicit bfloat adds. It's a storage type, and addition might not be what the user intended. So we should error.
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp | ||
---|---|---|
1045 | As I understand, as bfloat is a storage type, it should be converted explicitly to a different type if you want to do arithmetic operations. It makes sense to me to explicitly not allow this. |
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp | ||
---|---|---|
1045 | The frem is the only one in this list that would make sense for bf16. I believe the default would be Legal(?) So Expand seems like a sensible enough option. At least, it would seem a little odd to specifically mark them differently for bf16. We don't do anything special for fadd/fdiv/etc as far as I understand. |
clang-format not found in user's PATH; not linting file.