Implement for for signed integers (#15)

* implement `checked_binop`

* support `panic_nounwind`

* bless `for_range_signed`

* misc cleanups

Author: fee1-dead

crates/rustc_codegen_spirv/src/builder/builder_methods.rs

@@ -1318,28 +1318,80 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
     fn checked_binop(
         &mut self,
         oop: OverflowOp,
-        _ty: Ty<'_>,
+        ty: Ty<'_>,
         lhs: Self::Value,
         rhs: Self::Value,
     ) -> (Self::Value, Self::Value) {
-        // NOTE(eddyb) this needs to be `undef`, not `false`/`true`, because
-        // we don't want the user's boolean constants to keep the zombie alive.
-        let bool = SpirvType::Bool.def(self.span(), self);
-        let overflowed = self.undef(bool);
-        let result = match oop {
-            OverflowOp::Add => (self.add(lhs, rhs), overflowed),
-            OverflowOp::Sub => (self.sub(lhs, rhs), overflowed),
-            OverflowOp::Mul => (self.mul(lhs, rhs), overflowed),
+        // adopted partially from https://github.com/ziglang/zig/blob/master/src/codegen/spirv.zig
+        let is_add = match oop {
+            OverflowOp::Add => true,
+            OverflowOp::Sub => false,
+            OverflowOp::Mul => {
+                // NOTE(eddyb) this needs to be `undef`, not `false`/`true`, because
+                // we don't want the user's boolean constants to keep the zombie alive.
+                let bool = SpirvType::Bool.def(self.span(), self);
+                let overflowed = self.undef(bool);
+
+                let result = (self.mul(lhs, rhs), overflowed);
+                self.zombie(result.1.def(self), "checked mul is not supported yet");
+                return result;
+            }
         };
-        self.zombie(
-            result.1.def(self),
-            match oop {
-                OverflowOp::Add => "checked add is not supported yet",
-                OverflowOp::Sub => "checked sub is not supported yet",
-                OverflowOp::Mul => "checked mul is not supported yet",
-            },
-        );
-        result
+        let signed = match ty.kind() {
+            ty::Int(_) => true,
+            ty::Uint(_) => false,
+            other => self.fatal(format!(
+                "Unexpected {} type: {other:#?}",
+                match oop {
+                    OverflowOp::Add => "checked add",
+                    OverflowOp::Sub => "checked sub",
+                    OverflowOp::Mul => "checked mul",
+                }
+            )),
+        };
+
+        let result = if is_add {
+            self.add(lhs, rhs)
+        } else {
+            self.sub(lhs, rhs)
+        };
+
+        let overflowed = if signed {
+            // when adding, overflow could happen if
+            // - rhs is positive and result < lhs; or
+            // - rhs is negative and result > lhs
+            // this is equivalent to (rhs < 0) == (result > lhs)
+            //
+            // when subtracting, overflow happens if
+            // - rhs is positive and result > lhs; or
+            // - rhs is negative and result < lhs
+            // this is equivalent to (rhs < 0) == (result < lhs)
+            let rhs_lt_zero = self.icmp(IntPredicate::IntSLT, rhs, self.constant_int(rhs.ty, 0));
+            let result_gt_lhs = self.icmp(
+                if is_add {
+                    IntPredicate::IntSGT
+                } else {
+                    IntPredicate::IntSLT
+                },
+                result,
+                lhs,
+            );
+            self.icmp(IntPredicate::IntEQ, rhs_lt_zero, result_gt_lhs)
+        } else {
+            // for unsigned addition, overflow occured if the result is less than any of the operands.
+            // for subtraction, overflow occured if the result is greater.
+            self.icmp(
+                if is_add {
+                    IntPredicate::IntULT
+                } else {
+                    IntPredicate::IntUGT
+                },
+                result,
+                lhs,
+            )
+        };
+
+        (result, overflowed)
     }
 
     // rustc has the concept of an immediate vs. memory type - bools are compiled to LLVM bools as
@@ -2766,8 +2818,9 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
                         kind: SpirvValueKind::Def(b_id),
                         ..
                     },
-                    _, // `&'static panic::Location<'static>`
-                ] = args[..]
+                    // NOTE(fee1-dead): the standard `panic` takes in a `Location` due to `track_caller`.
+                    // but for `panic_nounwind` it does not, therefore we only look at the first two arguments.
+                ] = args[..2]
                 {
                     if let Some(const_msg) = const_str_as_utf8(&[a_id, b_id]) {
                         decoded_format_args.const_pieces = Some([const_msg].into_iter().collect());

crates/rustc_codegen_spirv/src/builder/intrinsics.rs

@@ -122,7 +122,7 @@ impl<'a, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'a, 'tcx> {
 
             sym::saturating_add => {
                 assert_eq!(arg_tys[0], arg_tys[1]);
-                let result = match &arg_tys[0].kind() {
+                let result = match arg_tys[0].kind() {
                     TyKind::Int(_) | TyKind::Uint(_) => {
                         self.add(args[0].immediate(), args[1].immediate())
                     }

crates/rustc_codegen_spirv/src/codegen_cx/constant.rs

@@ -437,7 +437,7 @@ impl<'tcx> CodegenCx<'tcx> {
         // println!(
         //     "Creating const alloc of type {} with {} bytes",
         //     self.debug_type(ty),
-        //     alloc.len()
+        //     alloc.inner().len()
         // );
         let mut offset = Size::ZERO;
         let result = self.read_from_const_alloc(alloc, &mut offset, ty);

crates/rustc_codegen_spirv/src/codegen_cx/declare.rs

@@ -181,6 +181,7 @@ impl<'tcx> CodegenCx<'tcx> {
         if [
             self.tcx.lang_items().panic_fn(),
             self.tcx.lang_items().panic_fmt(),
+            self.tcx.lang_items().panic_nounwind(),
         ]
         .contains(&Some(def_id))
         {

tests/README.md

@@ -9,9 +9,9 @@ whether it should succeed compilation, or fail. If it is expected to fail, there
 The `src` folder here is the tool that iterates over every file in the `ui` folder. It uses the
 `compiletests` library, taken from rustc's own compiletest framework.
 
-You can run compiletests via `cargo compiletests`. This is an alias set up in `.cargo/config` for
+You can run compiletests via `cargo compiletest`. This is an alias set up in `.cargo/config` for
 `cargo run --release -p compiletests --`. You can filter to run specific tests by passing the
-(partial) filenames to `cargo compiletests some_file_name`, and update the `.stderr` files to
+(partial) filenames to `cargo compiletest some_file_name`, and update the `.stderr` files to
 contain new output via the `--bless` flag (with `--bless`, make sure you're actually supposed to be
 changing the .stderr files due to an intentional change, and hand-validate the output is correct
 afterwards).

tests/ui/lang/control_flow/for_range_signed-broken.rs

@@ -0,0 +1,8 @@
+// build-pass
+
+use spirv_std::spirv;
+
+#[spirv(fragment)]
+pub fn main(#[spirv(flat)] i: i32) {
+    for _ in 0..i {}
+}