t-crest / patmos-llvm

Port of the LLVM compiler infrastructure to the time-predictable processor Patmos
Other
15 stars 8 forks source link

Function splitter accepts inline assembly blocks larger than subfunction size #30

Closed Emoun closed 3 years ago

Emoun commented 3 years ago

Take this program:

define i32 @main() {
entry:
  ; We make a block of 64 bytes, which can in no way fit in a 32 byte subfunction
  %0 = call i32 asm "
        li      $0  = 4096  # Long arithmetic, 8 bytes each
        li      $0  = 4096  
        li      $0  = 4096  
        li      $0  = 4096  
        li      $0  = 4096  
        li      $0  = 4096  
        li      $0  = 4096  
        li      $0  = 4096  
    ", "=r"
    ()
  ret i32 %0
}

If we try to compile it with a 32 bytes method cache (llc main.ll -mpatmos-max-subfunction-size=32), it should fail, as we should not split inside an inline assembly block, and the block is twice the allowed size. However, this compiles successfully, without splitting the inline assembly block:

        .file   "<stdin>"
        .text
        .globl  main
        .align  16
        .type   main,@function
        .fstart main, .Ltmp0-main, 16
main:                                        # @main
# BB#0:
                brcfnd  .LBB0_1
.Ltmp0:
        .align  16
        .type   .LBB0_1,@code
        .size   .LBB0_1, .Ltmp1-.LBB0_1
        .fstart .LBB0_1, .Ltmp1-.LBB0_1, 16
.LBB0_1:
        #APP

                li              $r1     = 4096  # Long arithmetic, 8 bytes each
                li              $r1     = 4096
                li              $r1     = 4096
                li              $r1     = 4096
                li              $r1     = 4096
                li              $r1     = 4096
                li              $r1     = 4096
                li              $r1     = 4096

        #NO_APP
                brcfnd  .LBB0_2
.Ltmp1:
        .align  16
        .type   .LBB0_2,@code
        .size   .LBB0_2, .Ltmp2-.LBB0_2
        .fstart .LBB0_2, .Ltmp2-.LBB0_2, 16
.LBB0_2:
                retnd
.LBB0_3:                                     # %entry
                nop
.Ltmp2:
.Ltmp3:
        .size   main, .Ltmp3-main

This should be fixed, so that a compile error is thrown in such situations.

Emoun commented 3 years ago

Found the following code snippet in PatmosFunctionSplitter.cpp:

// should we check for i_size > MaxSize as well and issue a warning?
// => No, user should not get warnings if he cannot do anything about it

if (i_size > cache_size) {
  report_fatal_error("Inline assembly in function " +
                      MBB->getParent()->getFunction()->getName() +
                      " is larger than the method cache size!");
}

MaxSize here refers to the value of -mpatmos-max-subfunction-size while i_size I think refers to the current size of a block.

We can see that we issue an error if the method cache size is lower than the assembly block size, but the comment seems to suggest not issuing an error for subfunction size was a conscious decision.

However, I don't agree with the sentiment that the user cannot do anything about the subfunction size. He can redesign the inline assembly to be smaller or split it into multiple blocks, or he can change the value of the flag.