Page MenuHomeFreeBSD

stand/ficl 64-bit compatibility
ClosedPublic

Authored by bdragon on Tue, Sep 8, 11:37 PM.

Details

Summary

Currently, the only thing that prevents a functioning 64-bit ficl build is a few integer types that were intended to be fixed-width.

Changing them to C99 integer types allows building a functioning 64-bit ficl.

I tested this a while back locally the last time I was working on a Petitboot loader.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

bdragon created this revision.Tue, Sep 8, 11:37 PM
bdragon requested review of this revision.Tue, Sep 8, 11:37 PM
imp accepted this revision.Wed, Sep 9, 1:10 AM
imp added inline comments.
stand/ficl/ficl.h
252 ↗(On Diff #76787)

This won't work quite right for long long types though. Eg, it won't be all 1s

This revision is now accepted and ready to land.Wed, Sep 9, 1:10 AM
bdragon added inline comments.Wed, Sep 9, 9:06 PM
stand/ficl/ficl.h
252 ↗(On Diff #76787)

I believe that since FICL_INT is actually required to be equal to the size of void*, this is not an issue in practice.

tsoome added inline comments.Wed, Sep 9, 9:09 PM
stand/ficl/ficl.h
252 ↗(On Diff #76787)

if words are 64-bit and long long actually 128, then forth has double words for 128 bit

bdragon added inline comments.Wed, Sep 9, 9:13 PM
stand/ficl/ficl.h
252 ↗(On Diff #76787)

Yes, but that is a multiple-cell situation. The entire FICL codebase assumes that cells are either 32 bit or 64 bit and will not build if this is not the case.

tsoome added inline comments.Wed, Sep 9, 9:18 PM
stand/ficl/ficl.h
252 ↗(On Diff #76787)

Yes, exactly.

bdragon added inline comments.Wed, Sep 9, 9:28 PM
stand/ficl/ficl.h
252 ↗(On Diff #76787)

By the way, here are the overrides that these changes are in support of:

CFLAGS+=        -DFICL_INT=int64_t -DFICL_UNS=uint64_t -DBITS_PER_CELL=64 -DFICL_ALIGN=3

(for ficl running on powerpc64(le) in 64 bit mode)

Actually, reading this over again, I think the FICL_INT part is wrong too, it should have been FICL_UNS.

I guess I'll be adjusting this after all.

imp added inline comments.Wed, Sep 9, 11:41 PM
stand/ficl/ficl.h
252 ↗(On Diff #76787)

You need to cast the 0 to the proper type BEFORE ~ to get the right answer when FILC_INT is larger than long.

That's the point I'm trying to make. If there's casts needed for other reasons, you need to do those too.

bdragon updated this revision to Diff 76986.Sun, Sep 13, 6:29 PM

Addressed comments.

This version passes make tinderbox locally, and hardware test passed on G4 (memmap command works as expected)

This revision now requires review to proceed.Sun, Sep 13, 6:29 PM
bdragon marked 4 inline comments as done.Sun, Sep 13, 6:30 PM
tsoome added inline comments.Sun, Sep 13, 7:29 PM
stand/powerpc/ofw/main.c
66 ↗(On Diff #76986)

wouldn't it be more correct to use uintptr_t here?

bdragon updated this revision to Diff 76987.Sun, Sep 13, 10:21 PM

Address review comment

bdragon marked an inline comment as done.Sun, Sep 13, 10:22 PM
tsoome accepted this revision.Mon, Sep 14, 4:42 AM
This revision is now accepted and ready to land.Mon, Sep 14, 4:42 AM
This revision was automatically updated to reflect the committed changes.