Skip to content

Fix fastcall not applying inreg attributes to arguments #38542

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 26, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/librustc_llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ pub enum Attribute {
StructRet = 16,
UWTable = 17,
ZExt = 18,
InReg = 19,
}

/// LLVMIntPredicate
Expand Down
14 changes: 11 additions & 3 deletions src/librustc_trans/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ mod attr_impl {
// The subset of llvm::Attribute needed for arguments, packed into a bitfield.
bitflags! {
#[derive(Default, Debug)]
flags ArgAttribute : u8 {
flags ArgAttribute : u16 {
const ByVal = 1 << 0,
const NoAlias = 1 << 1,
const NoCapture = 1 << 2,
Expand All @@ -67,6 +67,7 @@ mod attr_impl {
const SExt = 1 << 5,
const StructRet = 1 << 6,
const ZExt = 1 << 7,
const InReg = 1 << 8,
}
}
}
Expand All @@ -80,7 +81,7 @@ macro_rules! for_each_kind {
impl ArgAttribute {
fn for_each_kind<F>(&self, mut f: F) where F: FnMut(llvm::Attribute) {
for_each_kind!(self, f,
ByVal, NoAlias, NoCapture, NonNull, ReadOnly, SExt, StructRet, ZExt)
ByVal, NoAlias, NoCapture, NonNull, ReadOnly, SExt, StructRet, ZExt, InReg)
}
}

Expand Down Expand Up @@ -573,7 +574,14 @@ impl FnType {
}

match &ccx.sess().target.target.arch[..] {
"x86" => cabi_x86::compute_abi_info(ccx, self),
"x86" => {
let flavor = if abi == Abi::Fastcall {
cabi_x86::Flavor::Fastcall
} else {
cabi_x86::Flavor::General
};
cabi_x86::compute_abi_info(ccx, self, flavor);
},
"x86_64" => if abi == Abi::SysV64 {
cabi_x86_64::compute_abi_info(ccx, self);
} else if abi == Abi::Win64 || ccx.sess().target.target.options.is_like_windows {
Expand Down
50 changes: 49 additions & 1 deletion src/librustc_trans/cabi_x86.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@ use type_::Type;
use super::common::*;
use super::machine::*;

pub fn compute_abi_info(ccx: &CrateContext, fty: &mut FnType) {
#[derive(PartialEq)]
pub enum Flavor {
General,
Fastcall
}

pub fn compute_abi_info(ccx: &CrateContext, fty: &mut FnType, flavor: Flavor) {
if !fty.ret.is_ignore() {
if fty.ret.ty.kind() == Struct {
// Returning a structure. Most often, this will use
Expand Down Expand Up @@ -51,4 +57,46 @@ pub fn compute_abi_info(ccx: &CrateContext, fty: &mut FnType) {
arg.extend_integer_width_to(32);
}
}

if flavor == Flavor::Fastcall {
// Mark arguments as InReg like clang does it,
// so our fastcall is compatible with C/C++ fastcall.

// Clang reference: lib/CodeGen/TargetInfo.cpp
// See X86_32ABIInfo::shouldPrimitiveUseInReg(), X86_32ABIInfo::updateFreeRegs()

// IsSoftFloatABI is only set to true on ARM platforms,
// which in turn can't be x86?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a reasonable deduction.


let mut free_regs = 2;

for arg in &mut fty.args {
if arg.is_ignore() || arg.is_indirect() { continue; }

if arg.ty.kind() == Float {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this meant to apply to both f32 and f64 by any chance? LLVM's float is our f32.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, the easiest way is writing a test function which has a double as a first argument and then two more ints and comparing current Rust and clang LLVM output.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I can tell you for a fact that Float there is f32 - there's a Double variant for f64.

continue;
}

let size = llbitsize_of_real(ccx, arg.ty);
let size_in_regs = (size + 31) / 32;

if size_in_regs == 0 {
continue;
}

if size_in_regs > free_regs {
break;
}

free_regs -= size_in_regs;

if size <= 32 && (arg.ty.kind() == Pointer || arg.ty.kind() == Integer) {
arg.attrs.set(ArgAttribute::InReg);
}

if free_regs == 0 {
break;
}
}
}
}
2 changes: 2 additions & 0 deletions src/rustllvm/RustWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ from_rust(LLVMRustAttribute kind) {
return Attribute::UWTable;
case ZExt:
return Attribute::ZExt;
case InReg:
return Attribute::InReg;
default:
llvm_unreachable("bad AttributeKind");
}
Expand Down
1 change: 1 addition & 0 deletions src/rustllvm/rustllvm.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ enum LLVMRustAttribute {
StructRet = 16,
UWTable = 17,
ZExt = 18,
InReg = 19,
};

typedef struct OpaqueRustString *RustStringRef;
Expand Down
85 changes: 85 additions & 0 deletions src/test/codegen/fastcall-inreg.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Checks if the "fastcall" calling convention marks function arguments
// as "inreg" like the C/C++ compilers for the platforms.
// x86 only.

// ignore-aarch64
// ignore-aarch64_be
// ignore-arm
// ignore-armeb
// ignore-avr
// ignore-bpfel
// ignore-bpfeb
// ignore-hexagon
// ignore-mips
// ignore-mipsel
// ignore-mips64
// ignore-mips64el
// ignore-msp430
// ignore-powerpc64
// ignore-powerpc64le
// ignore-powerpc
// ignore-r600
// ignore-amdgcn
// ignore-sparc
// ignore-sparcv9
// ignore-sparcel
// ignore-s390x
// ignore-tce
// ignore-thumb
// ignore-thumbeb
// ignore-x86_64 no-ignore-x86
// ignore-xcore
// ignore-nvptx
// ignore-nvptx64
// ignore-le32
// ignore-le64
// ignore-amdil
// ignore-amdil64
// ignore-hsail
// ignore-hsail64
// ignore-spir
// ignore-spir64
// ignore-kalimba
// ignore-shave
// ignore-wasm32
// ignore-wasm64

// compile-flags: -C no-prepopulate-passes

#![crate_type = "lib"]

mod tests {
// CHECK: @f1(i32 inreg, i32 inreg, i32)
#[no_mangle]
extern "fastcall" fn f1(_: i32, _: i32, _: i32) {}

// CHECK: @f2(i32* inreg, i32* inreg, i32*)
#[no_mangle]
extern "fastcall" fn f2(_: *const i32, _: *const i32, _: *const i32) {}

// CHECK: @f3(float, i32 inreg, i32 inreg, i32)
#[no_mangle]
extern "fastcall" fn f3(_: f32, _: i32, _: i32, _: i32) {}

// CHECK: @f4(i32 inreg, float, i32 inreg, i32)
#[no_mangle]
extern "fastcall" fn f4(_: i32, _: f32, _: i32, _: i32) {}

// CHECK: @f5(i64, i32)
#[no_mangle]
extern "fastcall" fn f5(_: i64, _: i32) {}

// CHECK: @f6(i1 inreg zeroext, i32 inreg, i32)
#[no_mangle]
extern "fastcall" fn f6(_: bool, _: i32, _: i32) {}
}