From 39512c272d0ff65181f73fd5a41e64ab5ffbe6c6 Mon Sep 17 00:00:00 2001 From: arti Date: Wed, 11 Nov 2026 17:36:15 +0000 Subject: [PATCH] tms34010: fix M2 data widths and FS0 after calls --- TMS34010.md | 49 ++++++++++++------- .../Target/TMS34010/TMS34010AsmPrinter.cpp | 15 ++++-- .../Target/TMS34010/TMS34010FrameLowering.cpp | 4 +- .../Target/TMS34010/TMS34010ISelLowering.cpp | 6 ++- llvm/lib/Target/TMS34010/TMS34010InstrInfo.td | 8 ++- .../Target/TMS34010/TMS34010TargetMachine.cpp | 3 +- llvm/test/CodeGen/TMS34010/m2-call-noret.ll | 16 ++++++ llvm/test/CodeGen/TMS34010/m2-data-i1.ll | 9 ++++ llvm/test/CodeGen/TMS34010/m2-data-z64.ll | 9 ++++ llvm/test/CodeGen/TMS34010/m2-fs1.ll | 8 ++- llvm/test/CodeGen/TMS34010/m2-i16.ll | 19 ++++++- .../test/CodeGen/TMS34010/m2-returned-call.ll | 2 +- 12 files changed, 117 insertions(+), 31 deletions(-) create mode 100644 llvm/test/CodeGen/TMS34010/m2-call-noret.ll create mode 100644 llvm/test/CodeGen/TMS34010/m2-data-i1.ll create mode 100644 llvm/test/CodeGen/TMS34010/m2-data-z64.ll diff --git a/TMS34010.md b/TMS34010.md index 1771b51de523..23163bcfe0af 100644 --- a/TMS34010.md +++ b/TMS34010.md @@ -22,10 +22,10 @@ The calling convention is the **TI ABI**: - Caller: if it passes arguments, save the old STK on the system stack with `MOVE A14, -*SP, 1`; push arguments on the program stack **in reverse order** (`MOVE , *A14+, 1`), each already a 32-bit slot; `CALLA`; then - re-establish the field size with `SETF 32, 0, 1`. -- Callee: set `SETF 32, 0, 1`; save the old FP and each used callee-saved - register on the system stack; `FP = STK`; allocate the frame with `ADDI - , STK`. The first explicit argument is at `FP(-32)`, the second at + re-establish both field sizes with `SETF 16, 0, 0` and `SETF 32, 0, 1`. +- Callee: set `SETF 16, 0, 0` and `SETF 32, 0, 1`; save the old FP and each + used callee-saved register on the system stack; `FP = STK`; allocate the + frame with `ADDI , STK`. The first explicit argument is at `FP(-32)`, the second at `FP(-64)`, and so on; locals are at positive FP offsets. - Epilogue: when the function has explicit arguments, restore STK from the caller's saved slot with `MOVE *SP((n+1)*32), A14, 1`; a no-argument @@ -38,25 +38,31 @@ The calling convention is the **TI ABI**: - Callee-saved registers are `A0`–`A7` and `A9`–`A12`. A value live across a call must survive in one of them (or a spill slot), because a call clobbers all other state except the reserved registers. -- `FS1` must be 32 at function entry and after every call. The prologue emits - `SETF 32, 0, 1`; `LowerCall` emits it immediately after each `CALLA`. +- `FS0` must be 16 (for `short` accesses) and `FS1` must be 32 (for 32-bit + words) at function entry and after every call. The prologue emits + `SETF 16, 0, 0` then `SETF 32, 0, 1`; `LowerCall` emits the same pair + immediately after each `CALLA`. Supported signatures are `i32` and pointer arguments, and `void`, `i32` or pointer returns. Narrow (`i8`/`i16`) parameters, `i64`, floats/aggregates, -varargs, `sret`/`byval`/`inreg`/`nest`, indirect calls and tail calls fail -loudly with an `M2 ...` diagnostic. Narrow loads/stores inside a function stay -supported (`MOVB`, FS0 `MOVE`). The optimizer's `returned` parameter attribute +varargs, `sret`/`byval`/`inreg`/`nest` and indirect calls fail loudly with an +`M2 ...` diagnostic. Ordinary `tail` markers are **demoted** to ordinary +calls (`LowerCall` clears `IsTailCall`); `musttail` is not implemented and +M2 C does not emit it. Real tail calls are M3. Narrow loads/stores inside a +function stay supported (`MOVB`, FS0 `MOVE`). The optimizer's `returned` parameter attribute is **accepted**: it is only an optimization hint and does not change the stack-slot ABI. `m2-returned.ll` pins that; `m2-reject-sret.ll` keeps the rejection of the ABI-affecting attributes. ### Implemented selection and printing -- `ADD` for `i32`; `MOVI` for constants; `SETF 32, 0, 1`; `RETS N`. +- `ADD` for `i32`; `MOVI` for constants; `SETF 16, 0, 0` and + `SETF 32, 0, 1`; `RETS N`. - `MOVE ..., 1` (FS1) for 32-bit load/store, `MOVE ..., 0` (FS0) for `i16`, sign-extending `MOVB` for `i8`. - Argument/return lowering with `PUSH32`, `POP32`, `PUSHARG`, `SETFP`, - `ADDSTK`, `MOVEFP`, `LOADSTK`, `SETF32`, `CALLA` and the `GETFI` pseudo. + `ADDSTK`, `MOVEFP`, `LOADSTK`, `SETF16`, `SETF32`, `CALLA` and the `GETFI` + pseudo. - Constant global addresses as `MOVI _sym+bits`; surviving GEP offsets are bit offsets (`ADDI 24`, not `ADDI 192`). - GSPA syntax: leading-underscore symbols, hex `...h` (or decimal), signed @@ -111,10 +117,16 @@ Width and alignment rules: - `.even` before a `.data` global whose alignment is at least 16 bits, so the label and the following `.word`/`.long` agree. This is required when a short string literal precedes an `int`: GSPA aligns the value, not the label. + `.even` is a 16-bit alignment only, so explicit `align 4`/`align 8` on + `.data` is not honored in M2; the target's standalone object alignment is + 2 bytes. A scalar's directive must match its width, or the next global's offset is -wrong. Private/local symbol names are sanitized to GSPA-safe names (every -non-alphanumeric/non-`_` character becomes `_HH`, keeping the leading `_`), so +wrong. Arrays accept only `i8`, `i16`, `i32` and pointer elements; an `i1` or +`i64` array (including `zeroinitializer`) is rejected rather than emitted at +the wrong size. Private/local symbol names are sanitized to GSPA-safe names +(every non-alphanumeric/non-`_` character becomes `_HH`, keeping the leading +`_`), so `.L.str` becomes a deterministic valid label. ELF directives (`.rodata`, `.asciz`, `.p2align`, `.section`) are never emitted. Unsupported initializer shapes fail with @@ -154,17 +166,18 @@ ninja -C build llc clang opt llvm-config FileCheck count not ```sh build/bin/llvm-lit -v build/test/CodeGen/TMS34010 --filter='m2-' -# -> 33 passed +# -> 37 passed ``` The tests are `llvm/test/CodeGen/TMS34010/m2-*.ll`: - Migrated/legacy selection: `m2-add`, `m2-load`, `m2-char`, `m2-i16`, `m2-store`, `m2-global`, `m2-late-gep`. -- ABI: `m2-args`, `m2-noret`, `m2-locals`, `m2-args-locals`, `m2-call`, - `m2-call3`, `m2-callee-save`, `m2-fs1`, `m2-spill`, `m2-returned`. -- Data: `m2-data`, `m2-data-avail`, `m2-data-align`, `m2-string`, - `m2-ptrinit`. +- ABI: `m2-args`, `m2-noret`, `m2-call-noret`, `m2-locals`, + `m2-args-locals`, `m2-call`, `m2-call3`, `m2-callee-save`, `m2-fs1`, + `m2-spill`, `m2-returned`, `m2-returned-call`. +- Data: `m2-data`, `m2-data-avail`, `m2-data-align`, `m2-data-z64`, + `m2-data-i1`, `m2-string`, `m2-ptrinit`. - Loud refusals: `m2-reject-i64`, `m2-reject-struct`, `m2-reject-vararg`, `m2-reject-sret`, `m2-reject-structret`, `m2-reject-arg8`, `m2-reject-i8ret`, `m2-reject-float`, `m2-reject-call-indirect`, diff --git a/llvm/lib/Target/TMS34010/TMS34010AsmPrinter.cpp b/llvm/lib/Target/TMS34010/TMS34010AsmPrinter.cpp index db7acb018878..d727d580eb5e 100644 --- a/llvm/lib/Target/TMS34010/TMS34010AsmPrinter.cpp +++ b/llvm/lib/Target/TMS34010/TMS34010AsmPrinter.cpp @@ -254,7 +254,11 @@ void TMS34010AsmPrinter::emitDataInitializer(const Constant *Init) { emitByteLines(Bytes); return; } - if (ElemTy->isIntegerTy() || ElemTy->isPointerTy()) { + // Only 16/32-bit integer and pointer elements have a GSPA directive that + // matches their width. Accepting i1 or i64 here would emit one .byte or + // .long per element and silently change the object size. + if (ElemTy->isIntegerTy(16) || ElemTy->isIntegerTy(32) || + ElemTy->isPointerTy()) { for (unsigned I = 0, E = CDA->getNumElements(); I != E; ++I) emitDataInitializer(CDA->getElementAsConstant(I)); return; @@ -275,7 +279,8 @@ void TMS34010AsmPrinter::emitDataInitializer(const Constant *Init) { emitByteLines(Bytes); return; } - if (ElemTy->isIntegerTy() || ElemTy->isPointerTy()) { + if (ElemTy->isIntegerTy(16) || ElemTy->isIntegerTy(32) || + ElemTy->isPointerTy()) { for (const Use &U : CA->operands()) emitDataInitializer(cast(U.get())); return; @@ -294,8 +299,10 @@ void TMS34010AsmPrinter::emitDataInitializer(const Constant *Init) { OutStreamer->emitRawText("\t.byte 0\n"); else if (Bits <= 16) OutStreamer->emitRawText("\t.word 0\n"); - else + else if (Bits <= 32) OutStreamer->emitRawText("\t.long 0\n"); + else + reportUnsupportedDataInitializer(); return; } if (Ty->isPointerTy()) { @@ -315,7 +322,7 @@ void TMS34010AsmPrinter::emitDataInitializer(const Constant *Init) { OutStreamer->emitRawText("\t.word 0\n"); return; } - if (ElemTy->isIntegerTy() || ElemTy->isPointerTy()) { + if (ElemTy->isIntegerTy(32) || ElemTy->isPointerTy()) { for (uint64_t I = 0; I != N; ++I) OutStreamer->emitRawText("\t.long 0\n"); return; diff --git a/llvm/lib/Target/TMS34010/TMS34010FrameLowering.cpp b/llvm/lib/Target/TMS34010/TMS34010FrameLowering.cpp index 56da90f2529f..55997201de13 100644 --- a/llvm/lib/Target/TMS34010/TMS34010FrameLowering.cpp +++ b/llvm/lib/Target/TMS34010/TMS34010FrameLowering.cpp @@ -54,7 +54,9 @@ void TMS34010FrameLowering::emitPrologue(MachineFunction &MF, DebugLoc DL = MBBI != MBB.end() ? MBBI->getDebugLoc() : DebugLoc(); // The field-size invariant comes first so that every frame access below is - // a 32-bit access. + // a 32-bit access. FS0 = 16 keeps a later `short` load/store correct even + // if the caller left it at 32. + BuildMI(MBB, MBBI, DL, TII.get(TMS34010::SETF16)); BuildMI(MBB, MBBI, DL, TII.get(TMS34010::SETF32)); // Save the caller's frame pointer, then the used callee-saved registers. diff --git a/llvm/lib/Target/TMS34010/TMS34010ISelLowering.cpp b/llvm/lib/Target/TMS34010/TMS34010ISelLowering.cpp index b6755e25430c..cdd034948751 100644 --- a/llvm/lib/Target/TMS34010/TMS34010ISelLowering.cpp +++ b/llvm/lib/Target/TMS34010/TMS34010ISelLowering.cpp @@ -179,8 +179,10 @@ SDValue TMS34010TargetLowering::LowerCall( RegMask, Chain), 0); - // ROM/Xlib code clobbers the field-size state, so re-establish FS1 = 32 - // before any further 32-bit access. + // ROM/Xlib code clobbers the field-size state, so re-establish both fields + // before any further access: FS0 = 16 for `short` and FS1 = 32 for words. + Chain = SDValue( + DAG.getMachineNode(TMS34010::SETF16, DL, MVT::Other, Chain), 0); Chain = SDValue( DAG.getMachineNode(TMS34010::SETF32, DL, MVT::Other, Chain), 0); diff --git a/llvm/lib/Target/TMS34010/TMS34010InstrInfo.td b/llvm/lib/Target/TMS34010/TMS34010InstrInfo.td index f0023ba855f5..11ddb535fda8 100644 --- a/llvm/lib/Target/TMS34010/TMS34010InstrInfo.td +++ b/llvm/lib/Target/TMS34010/TMS34010InstrInfo.td @@ -235,7 +235,13 @@ def LOADSTK : TMS34010Inst<(outs), (ins i32imm:$off), let Defs = [A14]; } -// The ABI's field-size invariant. No operands; side-effecting. +// The ABI's field-size pair. FS0 must be 16 for `short` accesses and FS1 +// must be 32 for words, so the prologue and every call re-establish both. +// ROM code may leave either field at a different width. +def SETF16 : TMS34010Inst<(outs), (ins), "SETF 16, 0, 0", []> { + let hasSideEffects = 1; +} + def SETF32 : TMS34010Inst<(outs), (ins), "SETF 32, 0, 1", []> { let hasSideEffects = 1; } diff --git a/llvm/lib/Target/TMS34010/TMS34010TargetMachine.cpp b/llvm/lib/Target/TMS34010/TMS34010TargetMachine.cpp index 5b7ae2fae9c0..e5b82c82a784 100644 --- a/llvm/lib/Target/TMS34010/TMS34010TargetMachine.cpp +++ b/llvm/lib/Target/TMS34010/TMS34010TargetMachine.cpp @@ -33,7 +33,8 @@ static const char *const TMS34010DataLayout = // GSPA 2.00 identifiers accept only alphanumerics and `_`; in particular a // private IR symbol such as `@.str` becomes `_.str`, which the assembler // rejects as an invalid opcode/expression. Encode every other character as -// `_HH` so the mapping stays deterministic and collision-free. +// `_HH` so the mapping is deterministic and GSPA-safe. It is not injective: +// `_` is left unchanged, so `@x.y` and `@x_2Ey` both map to `_x_2Ey`. static void sanitizeGSPASymbolName(SmallVectorImpl &Name) { bool NeedsSanitizing = false; for (char C : Name) { diff --git a/llvm/test/CodeGen/TMS34010/m2-call-noret.ll b/llvm/test/CodeGen/TMS34010/m2-call-noret.ll new file mode 100644 index 000000000000..d4e51eb45181 --- /dev/null +++ b/llvm/test/CodeGen/TMS34010/m2-call-noret.ll @@ -0,0 +1,16 @@ +; RUN: llc -march=tms34010 -O2 -filetype=asm < %s | FileCheck %s +; +; The caller saves the old STK only when it passes arguments. A no-argument +; call must not push it, even though the callee returns a value. +declare i32 @callee() +define i32 @call0() { +entry: + %r = call i32 @callee() + ret i32 %r +} +; CHECK-LABEL: _call0: +; CHECK-NOT: MOVE A14, -*SP, 1 +; CHECK: CALLA _callee +; CHECK: SETF 16, 0, 0 +; CHECK-NEXT: SETF 32, 0, 1 +; CHECK: RETS 0 diff --git a/llvm/test/CodeGen/TMS34010/m2-data-i1.ll b/llvm/test/CodeGen/TMS34010/m2-data-i1.ll new file mode 100644 index 000000000000..f905799a9f3e --- /dev/null +++ b/llvm/test/CodeGen/TMS34010/m2-data-i1.ll @@ -0,0 +1,9 @@ +; RUN: not llc -march=tms34010 -O0 -filetype=asm < %s 2>&1 | FileCheck %s +; RUN: not llc -march=tms34010 -O2 -filetype=asm < %s 2>&1 | FileCheck %s +; +; `[8 x i1] zeroinitializer` occupies one byte in the data layout, but there +; is no 1-bit GSPA directive. Emitting a .byte or .long per element would +; silently change the object size, so the printer must refuse it. +@b = private constant [8 x i1] zeroinitializer + +; CHECK: M2 .data supports only integer, pointer and byte-array initializers diff --git a/llvm/test/CodeGen/TMS34010/m2-data-z64.ll b/llvm/test/CodeGen/TMS34010/m2-data-z64.ll new file mode 100644 index 000000000000..314d43cdcc2a --- /dev/null +++ b/llvm/test/CodeGen/TMS34010/m2-data-z64.ll @@ -0,0 +1,9 @@ +; RUN: not llc -march=tms34010 -O0 -filetype=asm < %s 2>&1 | FileCheck %s +; RUN: not llc -march=tms34010 -O2 -filetype=asm < %s 2>&1 | FileCheck %s +; +; A `[2 x i64] zeroinitializer` is 16 bytes, but .data has no 64-bit +; directive. Emitting two .longs would silently shrink it and shift every +; following object, so the printer must refuse it. +@z64 = private global [2 x i64] zeroinitializer + +; CHECK: M2 .data supports only integer, pointer and byte-array initializers diff --git a/llvm/test/CodeGen/TMS34010/m2-fs1.ll b/llvm/test/CodeGen/TMS34010/m2-fs1.ll index 68f163563657..ee78314aaea9 100644 --- a/llvm/test/CodeGen/TMS34010/m2-fs1.ll +++ b/llvm/test/CodeGen/TMS34010/m2-fs1.ll @@ -1,4 +1,8 @@ ; RUN: llc -march=tms34010 -O2 -filetype=asm < %s | FileCheck %s +; +; A callee may leave any field size behind. The prologue and the call site +; must restore FS0 = 16 and FS1 = 32, in that order, before compiled code +; touches a `short` or a 32-bit word again. declare i32 @callee(i32) define i32 @caller(i32 %x) { entry: @@ -6,7 +10,9 @@ entry: ret i32 %r } ; CHECK-LABEL: _caller: -; CHECK: SETF 32, 0, 1 +; CHECK: SETF 16, 0, 0 +; CHECK-NEXT: SETF 32, 0, 1 ; CHECK: CALLA _callee +; CHECK-NEXT: SETF 16, 0, 0 ; CHECK-NEXT: SETF 32, 0, 1 ; CHECK: RETS 2 diff --git a/llvm/test/CodeGen/TMS34010/m2-i16.ll b/llvm/test/CodeGen/TMS34010/m2-i16.ll index 83ab35ca9179..845082addcae 100644 --- a/llvm/test/CodeGen/TMS34010/m2-i16.ll +++ b/llvm/test/CodeGen/TMS34010/m2-i16.ll @@ -1,6 +1,8 @@ ; RUN: llc -march=tms34010 -O2 -filetype=asm < %s | FileCheck %s -; FS0 selects 16-bit fields. The M1 harness must execute SETF 16,0,0 -; before calling this code (Task 4). +; +; FS0 selects 16-bit fields. The prologue and every call re-establish +; FS0 = 16 / FS1 = 32, so a `short` access is correct without harness setup, +; including one that follows a call. define i32 @loads(ptr %p) { %v = load i16, ptr %p %r = sext i16 %v to i32 @@ -11,7 +13,20 @@ define void @stores(ptr %p, i32 %v) { store i16 %t, ptr %p ret void } +declare i32 @callee(i32) +define void @store_after_call(ptr %p, i32 %v) { +entry: + %r = call i32 @callee(i32 %v) + %t = trunc i32 %r to i16 + store i16 %t, ptr %p + ret void +} ; CHECK-LABEL: _loads: ; CHECK: MOVE *A{{[0-9]+}}, A{{[0-9]+}}, 0 ; CHECK-LABEL: _stores: ; CHECK: MOVE A{{[0-9]+}}, *A{{[0-9]+}}, 0 +; CHECK-LABEL: _store_after_call: +; CHECK: CALLA _callee +; CHECK-NEXT: SETF 16, 0, 0 +; CHECK-NEXT: SETF 32, 0, 1 +; CHECK: MOVE A{{[0-9]+}}, *A{{[0-9]+}}, 0 diff --git a/llvm/test/CodeGen/TMS34010/m2-returned-call.ll b/llvm/test/CodeGen/TMS34010/m2-returned-call.ll index 17a0f767082f..6d6d928cfbab 100644 --- a/llvm/test/CodeGen/TMS34010/m2-returned-call.ll +++ b/llvm/test/CodeGen/TMS34010/m2-returned-call.ll @@ -11,7 +11,7 @@ entry: define i32 @call_ret(i32 %x) { entry: - %r = call i32 @ret_arg(i32 %x) + %r = call i32 @ret_arg(i32 returned %x) ret i32 %r }