Menu

#472 pic16: more long long impl, start moving to common stdlib

open
nobody
None
5
2024-03-11
2024-01-28
No

This snowballed a bit (sorry, let me know if you would rather me break these up to multiple tickets) but a lot more stuff is fixed for pic16, and I've started eliminating some of the duplicated stdlib and using the common implementation.

As I was refactoring the stdlib to fix more float/64-bit tests, it started running into other issues like FSR0H being optimized out incorrectly, bitwise and corrupting stack when a register had spilled, etc. Those weren't directly related but I didn't want to push anything that would introduce new failures, so those had to be fixed too.

  1. Loads of >4 bytes from temporary pointers work now (fixes some more tests). The calling convention of the generic pointer functions was improved to support this.
  2. Implement __memcpy
  3. Replace most of the stdlib headers and the floating-point math functions with those from the common stdlib. Fixes a handful of floating-point issues and adds more 64-bit library support. Folded a few minor details from pic16 into common headers but these changes were minimal.
  4. Add FSR0H as used in call/return so it isn't optimized out incorrectly. Describe generic pointer functions' custom calling convention as well.
  5. Enable stdout to USART in tests for pic16
  6. Optimize stdout to USART slightly for pic16
  7. Don't skip asmop cleanup when jumping due to a bitwise-and test (was corrupting stack when an asmop spilled and needed stack cleanup)
  8. -0.0 should be false, pic16 wasn't masking off sign bit
  9. Enable more float tests for pic16, they work now
  10. Add atoll() and isblank() (one test started failing since pic16 says it has long long now, but lacked those functions)

Unit test stats are improved again. next had:
Summary for 'pic16': 37 abnormal stops ( ), 818 failures, 20481 tests, 5908 test cases, 0 bytes, 0 ticks
With patches:
Summary for 'pic16': 36 abnormal stops ( ), 767 failures, 22142 tests, 5923 test cases, 0 bytes, 0 ticks

That was a lot of patches but I think it may have gotten over a hump where fixes were exposing other problems - hopefully more targeted patches from here on until the tests are all passing.

10 Attachments

Related

Discussion: [SDCC 4.4.0RC3] Does anyone have a basic 16F819 Blink led example for MPLABX V6.15?

Discussion

  • Philipp Klaus Krause

    Parts 1 to 3 are in [r14655].

     

    Related

    Commit: [r14655]

  • Philipp Klaus Krause

    Parts 7 to 10 are in [r14656].

     

    Related

    Commit: [r14656]

  • Philipp Klaus Krause

    Regarding patches 4, 5, and 6, I feel that I do not know the PIC devices, pic16 port and gpsim simulator well enough (basically I have no idea about any of the three) to make a decision on them.

     
  • Jonathon Hall

    Jonathon Hall - 2024-02-01

    Thanks for applying those @spth!

    How would you suggest we proceed for the remaining patches? My impression is that nobody else is active in the project any more that is familiar with PICs to review them. Unit test improvements are helpful but not perfect. I could go back through the history of those files and poke a few people here if you think we might get a response.

     
    • Benedikt Freisen

      I believe that the most promising approach would be to describe the intended changes in prose on the sdcc-user mailing list and to ask whether any of the active PIC users that might be lurking there consider any of the changes dangerous for their use cases.
      We could then wait for and collect feedback for a couple of weeks and proceed from there.

       
      • Jonathon Hall

        Jonathon Hall - 2024-02-07

        Thanks for the suggestion @roybaer! I sent a message to the mailing list and it awaits moderator approval :) Hope we get some feedback!

         
        • Benedikt Freisen

          I believe that that only happens if you are not already subscribed to the list.
          If your message does not get through within the next 1-3 days or so, you could simply subscribe to the list and send it again. The list is not terribly active and therefore won't spam your inbox.

           
  • danielt3

    danielt3 - 2024-02-08

    For patch 0005: I'm pretty much OK with it. I just have to test to see logs getting spitted out (I'm a big fan of logs, btw).
    For patch 0006: the question is whether the layout of the bits of the PIR1 control register is always the same. Maybe we can add a define for the particular bit to be tested for each model and redefine it properly. Other than that, I'm ok with it.

     

    Last edit: danielt3 2024-02-08
    • Jonathon Hall

      Jonathon Hall - 2024-02-26

      Thanks for having a look danielt3. Sorry for the silence, kept thinking I would be able to get back to this but just haven't been able to carve out any time. I believe PIR1 is the same on all devices but I will look over that more thoroughly, that's a good thought.

      Did you look over patch #4? That one is pretty important to ensure that it doesn't discard needed register writes thinking they are unneeded.

       
    • Jonathon Hall

      Jonathon Hall - 2024-03-09

      I looked over the PIR1 and TXSTA registers across all pic18 devices by checking the headers in sdcc, with a script.

      Bottom line, all of them do have PIR1.TXIF in the same bit position.

      Some headers don't have all the bits labeled for some reason, but I checked datasheets for some of those and they are there. PIR1.TXIF was missing in the 18F2331 family (8 devices), those are all covered by the same datasheet. For some reason the bit is labeled TBIF in the header, and the other bits' labels are missing.

      TXSTA.TRMT (used by the old impl) was missing for 179 devices, so I couldn't check all of these. These headers only labeled bit 6, either TX91 or NOT_TX8. I checked a few of each and TRMT was there (and with the patch we won't actually use this any more anyway).

      I'm not sure why the headers don't align with the datasheets, but for the moment I'm going to focus on one problem at a time :) I don't think we need to change this per-device.

      Make sense to you @danielt3?

       
      • Philipp Klaus Krause

        AFAIK, the headers are autogenerated from files in gputils.

         

Log in to post a comment.