[CIR][CodeGen][Lowering] Support Integer overflow with fwrap #539
Conversation
|
Since |
well, maybe .. but I would not introduce new operations for math arithmetic, at least now, if there is a chance to do the same with the existing ones. @bcardosolopes I didn't have a chance to check carefully, but as far I understand unsigned wrap is a standard behavior in C, and only in the rare case we need to emphasize there is no wrap with |
|
Mistake from my review: my intention was to suggest
+1
We need testcases that cover that too. By looking at the PR, seems like this is a TU level configuration, and emitting these flags is dependent on:
So here's one idea, perhaps we should encode that at the module level (like we for fp behavior) and LLVM lowering could then look at these module properties to pick how it should lower BinOps. How does that sound? For this to work, I'm assuming that all of these binops on both signed and unsigned in a TU will have to follow the rules, do you know if that applies? |
|
In fact we already have those global definitions, see |
|
@bcardosolopes
Just if you are curious about, how it's possible: compiled with NUW - no unsigned wrap |
|
|
||
| let assemblyFormat = [{ | ||
| `(` $kind `,` $lhs `,` $rhs `)` `:` type($lhs) attr-dict | ||
| `(` $kind `,` $lhs `,` $rhs `)` | ||
| (`nsw` $hasNoSignedWrap^)? |
There was a problem hiding this comment.
Add a verifier to make sure both nsw and nuw are only valid for the binops that make sense. It might also be needed to check for valid types: are those allowed with vectors or (recently introduced) extInts?
There was a problem hiding this comment.
This allowed with any integer type - and wtth extInts as well (checked). But is no allowed with vectors
|
Now the question is - do you want me to do the same for |
It'd be great if you can already fix the shift, so we don't need to revisit this - feel free to do it in another PR though, this one does good incremental work. |
|
|
||
| if (noWrap && !noWrapOps) | ||
| return emitError() << "The nsw/nuw flags are applicable to Add, Sub and " | ||
| "Mul operations only"; |
There was a problem hiding this comment.
Thanks! Please add 'invalid.cir' tests and nit one the error message: The nsw/nuw flags are applicable to opcodes: 'add', 'sub' and 'mul'.
gitoleg
force-pushed
the
integer-overflow
branch
from
df47c7f to
e43c387
Compare
|
@bcardosolopes done! |
This PR fixes some cases when a program compiled with `-fwrapv` fails
with `NYI` .
Basically, the default behavior is no overlap:
```
void baz(int x, int y) {
int z = x - y;
}
```
LLVM IR (no CIR enabled):
```
%sub = sub nsw i32 %0, %1
```
and with `-fwrapv` :
```
%sub = sub i32 %0, %1
```
We need something similar in CIR. The only way I see how to implement it
is to add a couple of attributes to the `BinOp` to make things even with
the llvm dialect.
Well, are there any other ideas?
---------
Co-authored-by: Bruno Cardoso Lopes <[email protected]>
This PR fixes some cases when a program compiled with `-fwrapv` fails
with `NYI` .
Basically, the default behavior is no overlap:
```
void baz(int x, int y) {
int z = x - y;
}
```
LLVM IR (no CIR enabled):
```
%sub = sub nsw i32 %0, %1
```
and with `-fwrapv` :
```
%sub = sub i32 %0, %1
```
We need something similar in CIR. The only way I see how to implement it
is to add a couple of attributes to the `BinOp` to make things even with
the llvm dialect.
Well, are there any other ideas?
---------
Co-authored-by: Bruno Cardoso Lopes <[email protected]>
This PR fixes some cases when a program compiled with `-fwrapv` fails
with `NYI` .
Basically, the default behavior is no overlap:
```
void baz(int x, int y) {
int z = x - y;
}
```
LLVM IR (no CIR enabled):
```
%sub = sub nsw i32 %0, %1
```
and with `-fwrapv` :
```
%sub = sub i32 %0, %1
```
We need something similar in CIR. The only way I see how to implement it
is to add a couple of attributes to the `BinOp` to make things even with
the llvm dialect.
Well, are there any other ideas?
---------
Co-authored-by: Bruno Cardoso Lopes <[email protected]>
This PR fixes some cases when a program compiled with `-fwrapv` fails
with `NYI` .
Basically, the default behavior is no overlap:
```
void baz(int x, int y) {
int z = x - y;
}
```
LLVM IR (no CIR enabled):
```
%sub = sub nsw i32 %0, %1
```
and with `-fwrapv` :
```
%sub = sub i32 %0, %1
```
We need something similar in CIR. The only way I see how to implement it
is to add a couple of attributes to the `BinOp` to make things even with
the llvm dialect.
Well, are there any other ideas?
---------
Co-authored-by: Bruno Cardoso Lopes <[email protected]>
This PR fixes some cases when a program compiled with `-fwrapv` fails
with `NYI` .
Basically, the default behavior is no overlap:
```
void baz(int x, int y) {
int z = x - y;
}
```
LLVM IR (no CIR enabled):
```
%sub = sub nsw i32 %0, %1
```
and with `-fwrapv` :
```
%sub = sub i32 %0, %1
```
We need something similar in CIR. The only way I see how to implement it
is to add a couple of attributes to the `BinOp` to make things even with
the llvm dialect.
Well, are there any other ideas?
---------
Co-authored-by: Bruno Cardoso Lopes <[email protected]>
This PR fixes some cases when a program compiled with `-fwrapv` fails
with `NYI` .
Basically, the default behavior is no overlap:
```
void baz(int x, int y) {
int z = x - y;
}
```
LLVM IR (no CIR enabled):
```
%sub = sub nsw i32 %0, %1
```
and with `-fwrapv` :
```
%sub = sub i32 %0, %1
```
We need something similar in CIR. The only way I see how to implement it
is to add a couple of attributes to the `BinOp` to make things even with
the llvm dialect.
Well, are there any other ideas?
---------
Co-authored-by: Bruno Cardoso Lopes <[email protected]>
This PR fixes some cases when a program compiled with `-fwrapv` fails
with `NYI` .
Basically, the default behavior is no overlap:
```
void baz(int x, int y) {
int z = x - y;
}
```
LLVM IR (no CIR enabled):
```
%sub = sub nsw i32 %0, %1
```
and with `-fwrapv` :
```
%sub = sub i32 %0, %1
```
We need something similar in CIR. The only way I see how to implement it
is to add a couple of attributes to the `BinOp` to make things even with
the llvm dialect.
Well, are there any other ideas?
---------
Co-authored-by: Bruno Cardoso Lopes <[email protected]>
This PR fixes some cases when a program compiled with
-fwrapvfails withNYI.Basically, the default behavior is no overlap:
LLVM IR (no CIR enabled):
and with
-fwrapv:We need something similar in CIR. The only way I see how to implement it is to add a couple of attributes to the
BinOpto make things even with the llvm dialect.Well, are there any other ideas?