This is an archive of the discontinued LLVM Phabricator instance.

[X86] TCRETURNmi fix for 32bit platform
ClosedPublic

Authored by ztong0001 on Jan 26 2022, 8:56 PM.

Details

Summary

This fix is similar to 3cf3ffce240e("Fix the TCRETURNmi64 bug differently.")

after allocating register for index+base, we will only have one register left

This bug affects linux kernel compilation for x86 target. Error happens when compiling kmod_si476x_core.

clang complains:

error: ran out of registers during register allocation

The full command is:

clang -Wp,-MMD,drivers/mfd/.si476x-cmd.o.d -nostdinc -isystem /opt/toolchain/main/lib/clang/14.0.0/include -I./arch/x86/include -I./arch/x86/include/generated -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -DKERNEL -Qunused-arguments -fmacro-prefix-map=./= -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Werror=return-type -Wno-format-security -std=gnu89 -no-integrated-as --prefix=/usr/bin/ -Werror=unknown-warning-option -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx -fcf-protection=none -m32 -msoft-float -mregparm=3 -freg-struct-return -fno-pic -mstack-alignment=4 -march=atom -mtune=atom -mtune=generic -Wa,-mtune=generic32 -ffreestanding -Wno-sign-compare -fno-asynchronous-unwind-tables -fno-delete-null-pointer-checks -Wno-frame-address -Wno-address-of-packed-member -O2 -Wframe-larger-than=1024 -fno-stack-protector -Wno-format-invalid-specifier -Wno-gnu -mno-global-merge -Wno-unused-but-set-variable -Wno-unused-const-variable -fomit-frame-pointer -ftrivial-auto-var-init=pattern -fno-stack-clash-protection -falign-functions=32 -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wno-array-bounds -fno-strict-overflow -fno-stack-check -Werror=date-time -Werror=incompatible-pointer-types -Wno-initializer-overrides -Wno-format -Wno-sign-compare -Wno-format-zero-length -Wno-pointer-to-enum-cast -Wno-tautological-constant-out-of-range-compare -DKBUILD_MODFILE='"drivers/mfd/si476x-core"' -DKBUILD_BASENAME='"si476x_cmd"' -DKBUILD_MODNAME='"si476x_core"' -D__KBUILD_MODNAME=kmod_si476x_core -c -o drivers/mfd/si476x-cmd.o drivers/mfd/si476x-cmd.c


LLVM cannot compile the following code for x86 32bit target, the reason is tail call(TCRETURNmi) is using 2 registers for index+base and we want to use more than one registers for passing function args and that is impossible.
This fix is similar to 3cf3ffce240e("Fix the TCRETURNmi64 bug differently.").
We will only use tail call when it is using <=1 registers for passing args.

struct BIG_PARM {
	int ver;
};

static struct {
	int (*foo)  (struct BIG_PARM* a, void *b);
	int (*bar)  (struct BIG_PARM* a);
	int (*zoo0) (void);
	int (*zoo1) (void);
	int (*zoo2) (void);
	int (*zoo3) (void);
	int (*zoo4) (void);
} vtable[] = {
	[0] = {
		.foo = (int (*)(struct BIG_PARM* a, void *b))0xdeadbeef,
	},
};

int something(struct BIG_PARM *a, void* b) {
	return vtable[a->ver].foo(a,b);
}
$ clang -std=gnu89 -m32 -mregparm=3 -mtune=generic -fno-strict-overflow -O2 -c t0.c -o t0.c.o
error: ran out of registers during register allocation
1 error generated.

Diff Detail

Event Timeline

ztong0001 created this revision.Jan 26 2022, 8:56 PM
ztong0001 requested review of this revision.Jan 26 2022, 8:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 26 2022, 8:56 PM
craig.topper retitled this revision from TCRETURNmi fix for 32bit platform to [X86] TCRETURNmi fix for 32bit platform.Jan 26 2022, 9:17 PM
ztong0001 updated this revision to Diff 404123.EditedJan 28 2022, 12:06 PM

wrong version was uploaded, pervious version is not counting register correctly

ztong0001 edited the summary of this revision. (Show Details)Jan 31 2022, 10:29 AM
ztong0001 set the repository for this revision to rG LLVM Github Monorepo.Jan 31 2022, 7:21 PM
ztong0001 updated this revision to Diff 405029.Feb 1 2022, 12:01 PM

Allow using 3 registers when call target is using global constant
see baz() in test llvm-project/llvm/test/CodeGen/X86/hipe-cc.ll

ztong0001 updated this revision to Diff 405092.Feb 1 2022, 1:52 PM

fix crash on test

LLVM :: CodeGen/X86/sibcall-2.ll
LLVM :: CodeGen/X86/sibcall.ll

Tests missing.

Hi @lebedev.ri ,
Newbie here.
Would you recommend adding a new test case file or modify an existing test file under llvm-project/llvm/test/CodeGen/X86/
Thanks!

  • Tong

Tests missing.

pengfei added inline comments.Feb 2 2022, 5:35 AM
llvm/lib/Target/X86/X86InstrCompiler.td
1246

Why do we need to check N->getNumOperands()?

1247

Should we add 2 more tests for the 2 NumRegs = 3?

Hi Pengfei,
Thanks for you comment.
Please see my inline comments.
Thanks!

  • Tong
llvm/lib/Target/X86/X86InstrCompiler.td
1246

Hi Pengfei,
Thanks for you comment.
I see BasePtr could be a <FrameIndexSDNode> that has 0 operand and I'm not sure what else are there so I added this to proactively avoid crash when calling getOperand(0).

1247

Please correct me if I'm wrong, from my understanding the two NumRegs=3 case are covered already in the following tests:

Line 1244:
LLVM :: CodeGen/X86/sibcall-2.ll
LLVM :: CodeGen/X86/sibcall.ll

Line 1247:
LLVM:: CodeGen/X86/hipe-cc.ll

Without those two NumRegs=3 lines the above tests would fail.

pengfei added inline comments.Feb 3 2022, 8:05 AM
llvm/lib/Target/X86/X86InstrCompiler.td
1246

Shouldn't you check BasePtr->getNumOperands()?

1247

That's fair.

ztong0001 added inline comments.Feb 3 2022, 8:37 AM
llvm/lib/Target/X86/X86InstrCompiler.td
1246

Ouch! You are right. It should be BasePtr instead of N. I will submit a revised version.
Thanks!

ztong0001 updated this revision to Diff 405665.Feb 3 2022, 8:37 AM

fixed a bug checking number of operators

The pre-merge checks always fail but doesn't seems like it is caused by this patch.
I went over other people's pre-merge checks and there are similar errors.
I wonder premerge checks need to pass before this code can be picked up in main branch.

pengfei accepted this revision.Feb 8 2022, 11:08 PM

LGTM.

This revision is now accepted and ready to land.Feb 8 2022, 11:08 PM
ztong0001 added a comment.EditedFeb 8 2022, 11:46 PM

Thank you @pengfei ,
I'm reading the LLVM document it seems like I need to commit the change myself, but I don't have write privilege, could you help me commit?

My name and email: Tong Zhang <ztong0001@gmail.com>

Thanks!

  • Tong
This revision was landed with ongoing or failed builds.Feb 9 2022, 4:34 AM
This revision was automatically updated to reflect the committed changes.