numworks / epsilon

Modern graphing calculator operating system.
https://www.numworks.com/resources/engineering/software/
1.73k stars 460 forks source link

Add arithmetical and logical operations on integers: and, or, xor, etc. #146

Closed debrouxl closed 1 year ago

debrouxl commented 7 years ago

The usual logical operations on booleans and integers:

debrouxl commented 3 years ago
Logic = "Logic"
LogicalAnd = "a AND b"
LogicalBitClear = "Effacer le bit numéro b dans a"
LogicalBitFlip = "Inverser le bit numéro b dans a"
LogicalBitGet = "Obtenir le bit numéro b dans a"
LogicalBitSet = "Allumer le bit numéro b dans a"
LogicalBitsClear = "Effacer bits b ds a [a AND NOT(b)]"
LogicalNand = "a NAND b"
LogicalNor = "a NOR b"
LogicalNot = "NOT a"
LogicalOr = "a OR b"
LogicalShiftLeft = "Décalage logique gauche [a << s]"
LogicalShiftRightArithmetic = "Décalage arith. droite [a >>> s]"
LogicalShiftRight = "Décalage logique droite [a >> s]"
LogicalRotateLeft = "Rotation gauche de a par r bits"
LogicalRotateRight= "Rotation droite de a par r bits"
LogicalXnor = "a XNOR b"
LogicalXor = "a XOR b"
TwosComplementToBits = "Equivalent en complément à deux"

Might save a character in all translations by using [a AND NOT b] in LogicalBitsClear. I thought about that because none of the other descriptions have parentheses.

bmuessig commented 3 years ago

Might save a character in all translations by using [a AND NOT b] in LogicalBitsClear. I thought about that because none of the other descriptions have parentheses.

Personally, I find the term easier to understand than the description in this case.

On a side note, I have decided to leave the logic operator names in English. I'm working at a TechEd company and whenever we're dealing with digital logic, we have settled to use the English forms internationally, as many German universities have done the same before us.

joecrop commented 3 years ago

Good to know, I have translated the other languages with google translate. There is one more line that I forgot if you want to provide translations too:

ExplicitNumberOfBits = "Explicit number of bits"
bmuessig commented 3 years ago
ExplicitNumberOfBits = "Explicit number of bits"

Sure, that would be "Explizite Bitbreite".

Good to know, I have translated the other languages with google translate.

I can really recommend deepl.com. It has quite good AI based translation of most European languages and in my comparisons usually does much better than Google Translate.

Thanks for implementing the new functions. I can't wait to hopefully see them merged with an upcoming version.

debrouxl commented 3 years ago

ExplicitNumberOfBits = "Nombre indiqué de bits", I'd say, in that context.

joecrop commented 3 years ago

Hi Guys,

I have created a pull request with all the features we discussed: https://github.com/numworks/epsilon/pull/1756 Can you please review? Most of the Code is copy-paste, but there are two files that contain the key functionality and test cases:

  1. poincare/src/integer.cpp
  2. poincare/test/logic.cpp
artaxxx commented 3 years ago

Thanks for participating so much in this issue! Some of these features may be useful for high school users. I will think about the list we would like to add, but I think the best first step is to take something basic. For example, the third argument of the functions could be removed. This may be a little too expert and not necessarily necessary (except for the bit rotations but I don't think it will go on the firmware for the first step of this implementation). I will think about the wording of the name and legend of these functions and their arrangement in the toolbox menu. See you soon!

debrouxl commented 3 years ago

Removing the third argument makes the functions much less useful in my book :) And now that the work has been done, it makes little sense to spend extra time removing it.

The list of functions here is already fairly limited. The TI-68k series from 25 years ago, the most familiar calculators to me, and the Nspire series which succeeds these, have and/or/xor, not, rotate (with a second optional argument specifying the direction), shift (ditto). I'm sure giac and everything derived from it has proper functions as well. This leaves nand/nor/xnor, as well as the bit functions, on the table; the 68000, among others, has bit functions. Without these, one has to resort to simple, but more annoying, operations based on shifts or rotates.

artaxxx commented 3 years ago

But, what is exactly the purpose of having the third argument in and for instance?

debrouxl commented 3 years ago

The implementation uses the third argument to truncate the output to the specified number of bits, which is what I want to occur when and'ing larger values together - such values can be produced easily by not, shifts and rotates - then extract only a subset of the bits :)

I don't think that we want to go through the work of providing everything needed to build a full-blown ALU, with complete flag handling and everything else expected from a well-behaved ALU. However, I think that it's best to provide users better functionality than competitors have been offering users for over two decades.

artaxxx commented 3 years ago

But I have the feeling that it should be a separate function (maybe trunc), right? Maybe we could stay consistent with other languages (operator library in Python for example) for the syntax of this kind of functions.

joecrop commented 3 years ago

I agree that the 3rd argument will add confusion to most anyone that hasn't read the documentation. Here's my comments:

  1. The 3rd argument is implicit, you only set it if you need it, otherwise it defaults to 32 bits. So no-one will be burdened by it unless they want it.
  2. The and function doesn't need it, but there is no harm in having two and functions: and(a,b) and and(a,b,n). Most people will use the former and the later is there for "advanced" users that want to truncate for free.
  3. The toolbox menu only lists the implicit optioned functions, there is a sub-menu for functions that have the explicit num_bits parameter. Screen Shot 2021-01-28 at 8 22 22 AM
Ecco commented 3 years ago

An implicit third argument would work, but in general I'm not a big fan of hidden features.

Also, since we also have Python interpreter, we may not need to handle all the weird cases in the Calculation app.

joecrop commented 3 years ago

Also, since we also have Python interpreter, we may not need to handle all the weird cases in the Calculation app.

Can you elaborate on this please? How would python eliminate the problem of the third argument?

Also, I am really hoping there aren't any issues writing this in c++ because it has already been implemented.

Ecco commented 3 years ago

Can you elaborate on this please? How would python eliminate the problem of the third argument?

Well, my point is that Python kinds of eliminate the need for the entire feature altogether:

So the question really is about "what do we want to put in the Calculation app".

Also, I am really hoping there aren't any issues writing this in c++ because it has already been implemented.

Well, not per se. I think it makes sense to provide some binary/boolean functions in the Calculation app, but then it's really a compromise.

One problem I can see is that the PR currently adds a lot of code (and results in an additional 32 KB to the firmware), for a feature that I don't think is really critical.

bmuessig commented 3 years ago

One problem I can see is that the PR currently adds a lot of code (and results in an additional 32 KB to the firmware), for a feature that I don't think is really critical.

Considering that, I'd vouch for having only two arguments. If space is this much of a concern, NAND and NOR could also be removed. However, I'd really like to see the feature become reality, as I am using these all the time and I never found the Python feature to be particularly quick to use on Numworks. I've already done it on another calculator, before I have it ready on Python.

debrouxl commented 3 years ago

bmuessig brings a very important point about usability. Having this kind of basic functionality available in the Calculation app - just like it is on other calculator models, simply put - without having to switch to another app, and back, seems important to me as well.

The weight of the third argument handling could be a lesser contributor to the binary size increase than the weight all of these ExpressionNode-derived classes, I'd have to build the patched version to check. If that holds true, then:

Having to cut down on features because of limited hardware definitely has a fun part, I did my share of that even for the NumWorks calculator, but also an annoying part. "Life is short and Flash is full".

joecrop commented 3 years ago

1) So, off the cuff, it seems like removing 3 functions and merging 2 we are saving 10 of 31 total toolbox functions. So it is likely that we can go from 32k to 20k. Is that good enough? Doesn't seem like it...

2) Removing all of the functions that have a 3rd argument would not be as a significant change as you might think. 60% of the firmware increase is actually from text, the text would get no smaller, so you would be reducing the functions by 40%, which is actually less of a decrease than proposal 1. Plus, all integers would be hard coded to 32 bits :(

joecrop commented 3 years ago

Maybe instead of playing the recursive game of "is this small enough", @Ecco can you advise on what size would be acceptable? Then we can see what features we can squeeze in that limitation.

joecrop commented 3 years ago

I took my local database and did the following for experimentation:

Now, instead of an increase of 42kB, the increase is only 25.3 kB.

Is that acceptable?

bmuessig commented 3 years ago

Congratulations on halving the required code size!

Ecco commented 3 years ago

Hi @joecrop ! First of all, I'd like to apologize for us not having enough time to review your PR in a more timely fashion. We're super busy lately, and this feature, while great, is not part of our core roadmap.

My biggest problem here is that the firmware increase resulting from your PR is an order of magnitude higher than what I would have expected. Indeed, one could argue that, eventually, performing a xor is just one opcode! Even though that's a simplistic approach, menus aside, I would really expect that feature to add just a few KB, maybe not even one.

Now, looking at your PR #1756, I think I understand part of the problem. You created an independent ExpressionNode for each operand. That's probably a bad idea because it results in a lot of code duplication, and as an additional consequence, in a much larger binary.

Indeed, the "meat" of the PR is really just in integer.cpp, and is not that big. Note that there's some duplication in here too: it's a waste of memory to implement LogicalNand when you already have LogicalNot and LogicalAnd.

ExpressionNodes are used when manipulating the expression tree. But all the functions we're talking about here (nand, xor, and so on…) behave in a very similar way when it comes to expression handling in general (essentially, they can only be applied to an integer and would then output an integer). So I'm pretty sure that all of those nodes should rather be an alias to a single BinaryOperationNode. Kind of like we do for ln and log.

joecrop commented 3 years ago

Great idea! I'll take a crack at it. This will make the code way more readable too.

joecrop commented 3 years ago

Well, I moved all of the existing 31 functions into a single (much more manageable) file that all alias to BinaryOperationNode. The code is more concise now, Instead of a 42kB increase, without loosing features, we are now sitting at an increase of 33.6kB.

RedGl0w commented 3 years ago

Well, I moved all of the existing 31 functions into a single (much more manageable) file that all alias to BinaryOperationNode. The code is more concise now, Instead of a 42kB increase, without loosing features, we are now sitting at an increase of 33.6kB.

33.6kB is still way too much for n0100 :/

debrouxl commented 3 years ago

Given how close to the limit the N0100 is (unless one solders an additional Flash chip, but very few people do that), most nontrivial changes are too much for the poor little N0100, sadly.

I'll try to build the code and check the size of vtables, or select functions such as BinaryOperationNode::serialize(). LTO can sometimes do cool optimizations with e.g. inlining, but virtual functions and vtables can easily get in the way. There's a lot of code duplication, which isn't currently optimizable due to the way the code is written, in the prologue and epilogue of shallowReduce() functions in binary_operation.cpp.

nand, nor and xnor should probably still go away.

debrouxl commented 3 years ago

For now, I'm still trying to apply the patch :) Judging by the fact that the first commit in the PR is "Use semantic branches on setup-arm-toolchain" 0de5c89d9bb0ba7504aa4e21f18cfeeb35caa360 , the base commit would seem to be its parent e0dcdcea286f7f31b9fbee4d43456a2292ac5bc8 , but git apply'ing or git am'ing the combined patch fails royally. Removing the first 7 patches still yields a conflict on patch 08/38 on the "k_numberOfExpressions" line.

Could you rebase the patchset on the current master HEAD, 11ef4bd99692aa1e45bdb57539a2eec8506552cb , or newer if that changes in the meantime ?

joecrop commented 3 years ago

OK, rebase complete... that was painful. Hopefully it is easy for you to pull now. I'll start making these low-hanging changes tonight:

joecrop commented 3 years ago

OK, I gave up on the rebase, it kept on failing and it was easier for me to just create a new branch. I have closed the existing PR and created a new one here: https://github.com/numworks/epsilon/pull/1775

Same content, just much fewer commits to get there.

aamartin0000 commented 3 years ago

Regarding “shift left arithmetic”: There is a difference with sll if you implement the Overflow and Carryout flags, it would indicate when your value becomes too big. Conditions differ for signed vs unsigned integers. Basically if you shift a one past the sign bit or carryout bit, with a positive number, or shift a zero past the sign bit of a negative number, it’s overflow. (Note that you can’t just compare the sign of the output with the sign of the input.)

artaxxx commented 1 year ago

Fixed in version 20

cben commented 1 year ago

Slightly off-topic, but it was mentioned above Python makes this unnecessary — but how do you even type ^&|~ operators, for they are nowhere on the keyboard?
(and operator module is absent, at least as of epsilon 15 which still had unlocked bootloader)

Turns out you can emit characters in the python shell e.g. chr(38) returns '&' and then copy-paste them into the script editor. I've now copied this to a script, so I always have it at hand for copy-paste:

>>> print("".join(map(chr, range(32,127))))
 !"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz{|}~

BTW, Python has a non-intuitive but elegant solution to "how many bits" 2s-complement problem:
Negative numbers are treated as having infinite number of leading 1 bits :exploding_head:. Just as positive numbers have infinite number of leading 0s. Given any two integers, any bitwise operation returns something with "infinite prefix" of either all 0s or all 1s, so it's represantable by a single sign bit — just extend both integers to same representation length before computation and treat the sign bit as a regular bit.

mobluse commented 1 year ago

You can type &^| using Toolbox/Catalog in 19.4.0, but not ~ (tilde) AFAIK.