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

Implement WORD() function #555

Closed
wants to merge 1 commit into from
Closed

Implement WORD() function #555

wants to merge 1 commit into from

Conversation

Rangi42
Copy link
Contributor

@Rangi42 Rangi42 commented Aug 22, 2020

This returns the low 16 bits of a label or constant, as discussed in #rgbds.

make develop does not give any new warnings, only the same ones as before:

src/asm/asmy.y: warning: 1 shift/reduce conflict [-Wconflicts-sr]
include/extern/getopt.h:29:14: error: ‘optarg’ redeclared without dllimport attribute: previous dllimport ignored [-Werror=attributes]
include/extern/getopt.h:30:12: error: ‘optind’ redeclared without dllimport attribute: previous dllimport ignored [-Werror=attributes]
include/extern/getopt.h:30:20: error: ‘opterr’ redeclared without dllimport attribute: previous dllimport ignored [-Werror=attributes]
include/extern/getopt.h:30:28: error: ‘optopt’ redeclared without dllimport attribute: previous dllimport ignored [-Werror=attributes]
include/extern/getopt.h:30:36: error: ‘optreset’ redeclared without dllimport attribute: previous dllimport ignored [-Werror=attributes]

make checkpatch does not give any warnings.

Using WORD(-X) in place of $10000 - X in pokecrystal succeeds to make compare.

src/asm/rpn.c Outdated Show resolved Hide resolved
test/asm/dw-word.asm Show resolved Hide resolved
@pinobatch
Copy link
Member

For comparison, ca65 (and hence ca83) has these five functions:

  • .lobyte(x) or <x to extract bits 7-0
  • .hibyte(x) or >x to extract bits 15-8
  • .bankbyte(x) or ^x to extract bits 23-16
  • .loword(x) to extract bits 15-0
  • .hiword(x) to extract bits 31-16

N.B.: The .bankbyte function in ca65 is so called to reflect terminology used in WDC's documentation for the 65C816 CPU. (This CPU is used alongside SM83 in a Super NES with Super Game Boy.) It's not to be confused with .bank(x) which is used with mappers.

This returns the low 16 bits of a label or constant.
@Rangi42
Copy link
Contributor Author

Rangi42 commented Aug 22, 2020

@ISSOtm Would LOWWORD and HIGHWORD, or LOWORD and HIWORD, be preferable to only WORD? (A high-word function could be useful for those 32-bit middle-endian values you mentioned.)

@ISSOtm
Copy link
Member

ISSOtm commented Aug 22, 2020

Given that we have never needed WORD so far, I don't think they're useful as built-ins. (In fact, I'm thinking WORD might be better left to #201; HIGH and LOW having special meaning with regs, they make more sense as built-ins)

@Rangi42
Copy link
Contributor Author

Rangi42 commented Aug 22, 2020

I think the dw WORD(-Label) use case justifies a built-in WORD function, to replace instances of Label & $ffff or $10000 - Label. With user-defined functions, projects would end up each defining the same function if it doesn't already exist. (IIRC you were considering built-in dbw, dwb, dba, dab, and/or dn for similar reasons?)

@ISSOtm
Copy link
Member

ISSOtm commented Aug 22, 2020

No, only something that would make defining them easier.

Projects defining the same functions, to me, seems akin to hardware.inc; the community could create a rgblib.inc file, for example, containing wildely-agreed upon macros, for example.

@Rangi42
Copy link
Contributor Author

Rangi42 commented Aug 25, 2020

Shall I close this then?

@ISSOtm
Copy link
Member

ISSOtm commented Aug 25, 2020

Let's keep it around, if for any reason user-defined functions wind up not being implemented.

@Rangi42
Copy link
Contributor Author

Rangi42 commented Aug 31, 2020

Superseded by a PR from a feature branch, #561.

@Rangi42 Rangi42 closed this Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants