Skip to content

Commit

Permalink
ISLE: Common accessors for some insn data fields (#3781)
Browse files Browse the repository at this point in the history
Add accessors to prelude.isle to access data fields of
`func_addr` and `symbol_value` instructions.

These are based on similar versions I had added to the s390x
back-end, but are a bit more straightforward to use.

- func_ref_data: Extract SigRef, ExternalName, and RelocDistance
  fields given a FuncRef.

- symbol_value_data: Extract ExternalName, RelocDistance, and
  offset fields given a GlobalValue representing a Symbol.

- reloc_distance_near: Test for RelocDistance::Near.

The s390x back-end is changed to use these common versions.

Note that this exposed a bug in common isle code: This extractor:

(extractor (load_sym inst)
  (and inst
       (load _ (def_inst (symbol_value
                           (symbol_value_data _
                             (reloc_distance_near) offset)))
               (i64_from_offset
                 (memarg_symbol_offset_sum <offset _)))))

would raise an assertion in sema.rs due to a supposed cycle in
extractor definitions.  But there was no actual cycle, it was
simply that the extractor tree refers twice to the `insn_data`
extractor (once via the `load` and once via the `symbol_value`
extractor).  Fixed by checking for pre-existing definitions only
along one path in the tree, not across the whole tree.
  • Loading branch information
uweigand authored Feb 9, 2022
1 parent 9c5c872 commit 1019855
Show file tree
Hide file tree
Showing 13 changed files with 959 additions and 882 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
src/clif.isle 9ea75a6f790b5c03
src/prelude.isle 6aaf8ce0f5a5c2ec
src/prelude.isle 73285cd431346d53
src/isa/aarch64/inst.isle dafd813ba278ce19
src/isa/aarch64/lower.isle 2d2e1e076a0c8a23
22 changes: 14 additions & 8 deletions cranelift/codegen/src/isa/aarch64/lower/isle/generated_code.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 12 additions & 26 deletions cranelift/codegen/src/isa/s390x/inst.isle
Original file line number Diff line number Diff line change
Expand Up @@ -904,19 +904,6 @@
(extern extractor allow_div_traps allow_div_traps)


;; Helpers to access instruction data members ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

;; Extractor for `symbol_value` instruction data member.

(decl symbol_value_data (BoxExternalName RelocDistance i64) Inst)
(extern extractor symbol_value_data symbol_value_data)

;; Extractor for `call_target` instruction data members.

(decl call_target_data (BoxExternalName RelocDistance) Inst)
(extern extractor call_target_data call_target_data)


;; Helpers for register numbers and types ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

;; Hard-coded registers.
Expand Down Expand Up @@ -1090,13 +1077,6 @@

;; Helpers for memory arguments ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

;; Accessors for `RelocDistance`.

(type RelocDistance extern (enum))

(decl reloc_distance_near () RelocDistance)
(extern extractor reloc_distance_near reloc_distance_near)

;; Accessors for `Offset32`.

(decl zero_offset () Offset32)
Expand All @@ -1105,6 +1085,11 @@
(decl i64_from_offset (i64) Offset32)
(extern extractor infallible i64_from_offset i64_from_offset)

;; Accessors for `ExternalName`.

(decl box_external_name (ExternalName) BoxExternalName)
(extern constructor box_external_name box_external_name)

;; Accessors for `MemFlags`.

(decl littleendian () MemFlags)
Expand All @@ -1126,7 +1111,7 @@
(decl memarg_reg_plus_off (Reg i64 MemFlags) MemArg)
(extern constructor memarg_reg_plus_off memarg_reg_plus_off)

(decl memarg_symbol (BoxExternalName i32 MemFlags) MemArg)
(decl memarg_symbol (ExternalName i32 MemFlags) MemArg)
(extern constructor memarg_symbol memarg_symbol)

;; Form the sum of two offset values, and check that the result is
Expand All @@ -1149,7 +1134,7 @@
(memarg_reg_plus_reg (put_in_reg x) (put_in_reg y) flags))

(rule (lower_address flags
(def_inst (symbol_value_data name (reloc_distance_near) offset))
(def_inst (symbol_value (symbol_value_data name (reloc_distance_near) offset)))
(i64_from_offset (memarg_symbol_offset_sum <offset final_offset)))
(memarg_symbol name final_offset flags))

Expand All @@ -1159,13 +1144,13 @@
(decl load_sym (Inst) Inst)
(extractor (load_sym inst)
(and inst
(load _ (def_inst (symbol_value_data _ (reloc_distance_near) offset))
(load _ (def_inst (symbol_value (symbol_value_data _ (reloc_distance_near) offset)))
(i64_from_offset (memarg_symbol_offset_sum <offset _)))))

(decl uload16_sym (Inst) Inst)
(extractor (uload16_sym inst)
(and inst
(uload16 _ (def_inst (symbol_value_data _ (reloc_distance_near) offset))
(uload16 _ (def_inst (symbol_value (symbol_value_data _ (reloc_distance_near) offset)))
(i64_from_offset (memarg_symbol_offset_sum <offset _)))))


Expand Down Expand Up @@ -1730,10 +1715,11 @@
(SideEffectNoResult.Inst (MInst.FpuStoreRev64 src addr)))

;; Helper for emitting `MInst.LoadExtNameFar` instructions.
(decl load_ext_name_far (BoxExternalName i64) Reg)
(decl load_ext_name_far (ExternalName i64) Reg)
(rule (load_ext_name_far name offset)
(let ((dst WritableReg (temp_writable_reg $I64))
(_ Unit (emit (MInst.LoadExtNameFar dst name offset))))
(boxed_name BoxExternalName (box_external_name name))
(_ Unit (emit (MInst.LoadExtNameFar dst boxed_name offset))))
(writable_reg_to_reg dst)))

;; Helper for emitting `MInst.LoadAddr` instructions.
Expand Down
14 changes: 5 additions & 9 deletions cranelift/codegen/src/isa/s390x/lower.isle
Original file line number Diff line number Diff line change
Expand Up @@ -1156,27 +1156,23 @@
;;;; Rules for `func_addr` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

;; Load the address of a function, target reachable via PC-relative instruction.
(rule (lower (and (func_addr _)
(call_target_data name (reloc_distance_near))))
(rule (lower (func_addr (func_ref_data _ name (reloc_distance_near))))
(value_reg (load_addr (memarg_symbol name 0 (memflags_trusted)))))

;; Load the address of a function, general case.
(rule (lower (and (func_addr _)
(call_target_data name _)))
(rule (lower (func_addr (func_ref_data _ name _)))
(value_reg (load_ext_name_far name 0)))


;;;; Rules for `symbol_value` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

;; Load the address of a symbol, target reachable via PC-relative instruction.
(rule (lower (and (symbol_value _)
(symbol_value_data name (reloc_distance_near)
(memarg_symbol_offset offset))))
(rule (lower (symbol_value (symbol_value_data name (reloc_distance_near)
(memarg_symbol_offset offset))))
(value_reg (load_addr (memarg_symbol name offset (memflags_trusted)))))

;; Load the address of a symbol, general case.
(rule (lower (and (symbol_value _)
(symbol_value_data name _ offset)))
(rule (lower (symbol_value (symbol_value_data name _ offset)))
(value_reg (load_ext_name_far name offset)))


Expand Down
36 changes: 10 additions & 26 deletions cranelift/codegen/src/isa/s390x/lower/isle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ use crate::machinst::isle::*;
use crate::settings::Flags;
use crate::{
ir::{
condcodes::*, immediates::*, types::*, AtomicRmwOp, Endianness, ExternalName, Inst,
InstructionData, StackSlot, TrapCode, Value, ValueLabel, ValueList,
condcodes::*, immediates::*, types::*, AtomicRmwOp, Endianness, Inst, InstructionData,
StackSlot, TrapCode, Value, ValueLabel, ValueList,
},
isa::s390x::inst::s390x_map_regs,
isa::unwind::UnwindInst,
machinst::{InsnOutput, LowerCtx, RelocDistance},
machinst::{InsnOutput, LowerCtx},
};
use std::boxed::Box;
use std::cell::Cell;
Expand Down Expand Up @@ -127,18 +127,6 @@ where
}
}

#[inline]
fn symbol_value_data(&mut self, inst: Inst) -> Option<(BoxExternalName, RelocDistance, i64)> {
let (name, dist, offset) = self.lower_ctx.symbol_value(inst)?;
Some((Box::new(name.clone()), dist, offset))
}

#[inline]
fn call_target_data(&mut self, inst: Inst) -> Option<(BoxExternalName, RelocDistance)> {
let (name, dist) = self.lower_ctx.call_target(inst)?;
Some((Box::new(name.clone()), dist))
}

#[inline]
fn writable_gpr(&mut self, regno: u8) -> WritableReg {
super::writable_gpr(regno)
Expand Down Expand Up @@ -404,15 +392,6 @@ where
vec[usize::from(index)]
}

#[inline]
fn reloc_distance_near(&mut self, dist: &RelocDistance) -> Option<()> {
if *dist == RelocDistance::Near {
Some(())
} else {
None
}
}

#[inline]
fn zero_offset(&mut self) -> Offset32 {
Offset32::new(0)
Expand Down Expand Up @@ -443,6 +422,11 @@ where
}
}

#[inline]
fn box_external_name(&mut self, name: ExternalName) -> BoxExternalName {
Box::new(name)
}

#[inline]
fn memflags_trusted(&mut self) -> MemFlags {
MemFlags::trusted()
Expand All @@ -459,9 +443,9 @@ where
}

#[inline]
fn memarg_symbol(&mut self, name: BoxExternalName, offset: i32, flags: MemFlags) -> MemArg {
fn memarg_symbol(&mut self, name: ExternalName, offset: i32, flags: MemFlags) -> MemArg {
MemArg::Symbol {
name,
name: Box::new(name),
offset,
flags,
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
src/clif.isle 9ea75a6f790b5c03
src/prelude.isle 6aaf8ce0f5a5c2ec
src/isa/s390x/inst.isle 1ae3c0f9c956affd
src/isa/s390x/lower.isle d18ee0bff12cad4e
src/prelude.isle 73285cd431346d53
src/isa/s390x/inst.isle 87a2d7c0c69d0324
src/isa/s390x/lower.isle 3c124e26bc411983
Loading

0 comments on commit 1019855

Please sign in to comment.