Skip to content
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

kernel: use ffdata.h constants in finite field code #4870

Merged
merged 1 commit into from
Apr 20, 2022
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
8 changes: 8 additions & 0 deletions etc/ffgen.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@ void emit_code(FILE * dest, int header)
fprintf(dest, "extern const UInt1 DegrFF[NUM_SHORT_FINITE_FIELDS+1];\n");
fprintf(dest, "extern const UInt4 CharFF[NUM_SHORT_FINITE_FIELDS+1];\n");
fprintf(dest, "\n");
if (num_ff < 65536)
fprintf(dest, "typedef UInt2 FF;\n");
else
fprintf(dest, "typedef UInt4 FF;\n");
if (MAX_FF <= 65536)
fprintf(dest, "typedef UInt2 FFV;\n");
else
fprintf(dest, "typedef UInt4 FFV;\n");
fprintf(dest, "\n");
fprintf(dest, "#endif // GAP_FFDATA_H\n");
}
Expand Down
4 changes: 2 additions & 2 deletions hpcgap/lib/ffeconway.gi
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ InstallOtherMethod(ZOp,
function(p,d)
local q;
if not IsPrimeInt(p) then
Error("Z: <p> must be a prime");
Error("Z: <p> must be a prime (not the integer ", p, ")");
fi;
q := p^d;
if q <= MAXSIZE_GF_INTERNAL or d =1 then
Expand All @@ -191,7 +191,7 @@ InstallMethod(ZOp,
d := LogInt(q,p);
Assert(1, q=p^d);
if not IsPrimeInt(p) then
Error("Z: <q> must be a positive prime power");
Error("Z: <q> must be a positive prime power (not the integer ", q, ")");
fi;
if d > 1 then
return FFECONWAY.ZNC(p,d);
Expand Down
4 changes: 2 additions & 2 deletions lib/ffeconway.gi
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ InstallOtherMethod(ZOp,
function(p,d)
local q;
if not IsPrimeInt(p) then
Error("Z: <p> must be a prime");
Error("Z: <p> must be a prime (not the integer ", p, ")");
fi;
q := p^d;
if q <= MAXSIZE_GF_INTERNAL or d =1 then
Expand All @@ -178,7 +178,7 @@ InstallMethod(ZOp,
d := LogInt(q,p);
Assert(1, q=p^d);
if not IsPrimeInt(p) then
Error("Z: <q> must be a positive prime power");
Error("Z: <q> must be a positive prime power (not the integer ", q, ")");
fi;
if d > 1 then
return FFECONWAY.ZNC(p,d);
Expand Down
19 changes: 10 additions & 9 deletions src/finfield.c
Original file line number Diff line number Diff line change
Expand Up @@ -1419,10 +1419,10 @@ static Obj FuncZ(Obj self, Obj q)
FF ff; /* the finite field */

/* check the argument */
if ( (IS_INTOBJ(q) && (INT_INTOBJ(q) > 65536)) ||
(TNUM_OBJ(q) == T_INTPOS))
return CALL_1ARGS(ZOp, q);
if ((IS_INTOBJ(q) && (INT_INTOBJ(q) > MAXSIZE_GF_INTERNAL)) ||
(TNUM_OBJ(q) == T_INTPOS))
return CALL_1ARGS(ZOp, q);

if ( !IS_INTOBJ(q) || INT_INTOBJ(q)<=1 ) {
RequireArgument(SELF_NAME, q, "must be a positive prime power");
}
Expand All @@ -1445,20 +1445,21 @@ static Obj FuncZ2(Obj self, Obj p, Obj d)
if (ARE_INTOBJS(p, d)) {
ip = INT_INTOBJ(p);
id = INT_INTOBJ(d);
if (ip > 1 && id > 0 && id <= 16 && ip < 65536) {
if (ip > 1 && id > 0 && id <= DEGREE_LARGEST_INTERNAL_FF &&
ip <= MAXSIZE_GF_INTERNAL) {
id1 = id;
q = ip;
while (--id1 > 0 && q <= 65536)
while (--id1 > 0 && q <= MAXSIZE_GF_INTERNAL)
q *= ip;
if (q <= 65536) {
if (q <= MAXSIZE_GF_INTERNAL) {
/* get the finite field */
ff = FiniteField(ip, id);
ff = FiniteFieldBySize(q);

if (ff == 0 || CHAR_FF(ff) != ip)
RequireArgument(SELF_NAME, p, "must be a prime");

/* make the root */
return NEW_FFE(ff, (ip == 2 && id == 1 ? 1 : 2));
return NEW_FFE(ff, (q == 2) ? 1 : 2);
}
}
}
Expand Down
48 changes: 25 additions & 23 deletions src/finfield.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@
**
** Finite fields are an important domain in computational group theory
** because the classical matrix groups are defined over those finite fields.
** In GAP we support small finite fields with up to 65536 elements,
** larger fields can be realized as polynomial domains over smaller fields.
** The GAP kernel supports elements of finite fields up to some fixed size
** limit stored in MAXSIZE_GF_INTERNAL. To change this limit for 32 resp.
** 64 bit systems, edit `etc/ffgen.c`. Support for fields larger than this
** is implemented by the GAP library.
**
** Elements in small finite fields are represented as immediate objects.
**
Expand All @@ -24,10 +26,11 @@
** The least significant 3 bits of such an immediate object are always 010,
** flagging the object as an object of a small finite field.
**
** The next 13 bits represent the small finite field where the element lies.
** They are simply an index into a global table of small finite fields.
** The next group of FIELD_BITS_FFE bits represent the small finite field
** where the element lies. They are simply an index into a global table of
** small finite fields, which is constructed at build time.
**
** The most significant 16 bits represent the value of the element.
** The most significant VAL_BITS_FFE bits represent the value of the element.
**
** If the value is 0, then the element is the zero from the finite field.
** Otherwise the integer is the logarithm of this element with respect to a
Expand Down Expand Up @@ -69,10 +72,14 @@
**
** Small finite fields are represented by an index into a global table.
**
** Since there are only 6542 (prime) + 93 (nonprime) small finite fields,
** the index fits into a 'UInt2' (actually into 13 bits).
** Depending on the configuration it may be UInt2 or UInt4. The definition
** is in `ffdata.h` and is calculated by `etc/ffgen.c`
*/
typedef UInt2 FF;
GAP_STATIC_ASSERT(NUM_SHORT_FINITE_FIELDS <= (1<<(8*sizeof(FF))),
"NUM_SHORT_FINITE_FIELDS too large for type FF");

GAP_STATIC_ASSERT(FIELD_BITS_FFE + VAL_BITS_FFE + 3 <= 8*sizeof(Obj),
"not enough bits in type Obj to store internal FFEs");


/****************************************************************************
Expand Down Expand Up @@ -140,17 +147,11 @@ extern Obj SuccFF;
** Values of elements of small finite fields are represented by the
** logarithm of the element with respect to the root plus one.
**
** Since small finite fields contain at most 65536 elements, the value fits
** into a 'UInt2'.
**
** It may be possible to change this to 'UInt4' to allow small finite fields
** with more than 65536 elements. The macros and have been coded in
** such a way that they work without problems. The exception is 'POW_FFV'
** which will only work if the product of integers of type 'FFV' does not
** cause an overflow. And of course the successor table stored for a finite
** field will become quite large for fields with more than 65536 elements.
** Depending on the configuration, this type may be a UInt2 or UInt4.
** This type is actually defined in `ffdata.h` by `etc/ffgen.c`
*/
typedef UInt2 FFV;
GAP_STATIC_ASSERT(MAXSIZE_GF_INTERNAL <= (1<<(8*sizeof(FFV))),
"MAXSIZE_GF_INTERNAL too large for type FFV");

GAP_STATIC_ASSERT(sizeof(UInt) >= 2 * sizeof(FFV),
"Overflow possibility in POW_FFV");
Expand Down Expand Up @@ -288,8 +289,7 @@ EXPORT_INLINE FFV QUO_FFV(FFV a, FFV b, const FFV * f)
** in the range $0..order(f)-1$.
**
** Finally 'POW_FFV' may only be used if the product of two integers of the
** size of 'FFV' does not cause an overflow, i.e. only if 'FFV' is
** 'unsigned short'.
** size of 'FFV' does not cause an overflow.
**
** If the finite field element is 0 the power is also 0, otherwise we have
** $a^n ~ (z^{a-1})^n = z^{(a-1)*n} = z^{(a-1)*n % (o-1)} ~ (a-1)*n % (o-1)$
Expand Down Expand Up @@ -320,7 +320,7 @@ EXPORT_INLINE FFV POW_FFV(FFV a, UInt n, const FFV * f)
EXPORT_INLINE FF FLD_FFE(Obj ffe)
{
GAP_ASSERT(IS_FFE(ffe));
return (FF)((((UInt)(ffe)) & 0xFFFF) >> 3);
return (FF)((UInt)(ffe) >> 3) & ((1 << FIELD_BITS_FFE) - 1);
}


Expand All @@ -336,7 +336,8 @@ EXPORT_INLINE FF FLD_FFE(Obj ffe)
EXPORT_INLINE FFV VAL_FFE(Obj ffe)
{
GAP_ASSERT(IS_FFE(ffe));
return (FFV)(((UInt)(ffe)) >> 16);
return (FFV)((UInt)(ffe) >> (3 + FIELD_BITS_FFE)) &
((1 << VAL_BITS_FFE) - 1);
}


Expand All @@ -351,7 +352,8 @@ EXPORT_INLINE FFV VAL_FFE(Obj ffe)
EXPORT_INLINE Obj NEW_FFE(FF fld, FFV val)
{
GAP_ASSERT(val < SIZE_FF(fld));
return (Obj)(((UInt)(val) << 16) + ((UInt)(fld) << 3) + (UInt)0x02);
return (Obj)(((UInt)val << (3 + FIELD_BITS_FFE)) | ((UInt)fld << 3) |
(UInt)0x02);
}


Expand Down
10 changes: 6 additions & 4 deletions tst/testinstall/ffe.tst
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ gap> Z(-2);
Error, Z: <q> must be a positive prime power (not the integer -2)
gap> Z(6);
Error, Z: <q> must be a positive prime power (not the integer 6)
gap> Z(65537*65539);
Error, Z: <q> must be a positive prime power (not the integer 4295229443)

# variant with two arguments
gap> Z(0,1);
Expand Down Expand Up @@ -65,13 +67,13 @@ Error, Z: <p> must be a prime (not the integer 9)
gap> Z(9,2);
Error, Z: <p> must be a prime (not the integer 9)
gap> Z(2^16,1);
Error, Z: <p> must be a prime
Error, Z: <p> must be a prime (not the integer 65536)
gap> Z(2^16,2);
Error, Z: <p> must be a prime
Error, Z: <p> must be a prime (not the integer 65536)
gap> Z(2^17,1);
Error, Z: <p> must be a prime
Error, Z: <p> must be a prime (not the integer 131072)
gap> Z(2^17,2);
Error, Z: <p> must be a prime
Error, Z: <p> must be a prime (not the integer 131072)

# Invoking Z(p,d) with p not a prime used to crash gap, which we fixed.
# However, invocations like `Z(4,5)` still would erroneously trigger the
Expand Down