[PATCH] D84923: [ARM] Fix so immediates and pc relative checks

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Hey.

Constant island pass is notorious for subtle bugs and being difficult to test.

I created this test case that breaks, by iterating over all values of SPACE.

I think this is why Diogo would have added the PCOffset handling. The pass is usually conservative about the ends of the offset ranges. Now there is a point where we need to be exact where the SOImm range changes from being 2byte addressable to 4byte addressable.

Dave

1. RUN: llc --filetype=obj -start-before=arm-cp-islands -o - %s | \
2. RUN: llvm-objdump --arch=armv8a --disassemble - | FileCheck %s

- | target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64" target triple = "armv8.2a-arm-none-eabi"

  define dso_local i32 @main() #0 { ret i32 0 }

  attributes #0 = { "frame-pointer"="all" } !4 = !{i32 210}

...
---

name:            main
alignment:       4
tracksRegLiveness: true
constants:

- id:              0 value:           half 0xH5440 alignment:       2

machineFunctionInfo: {}
body:             |

  bb.0 (%ir-block.0):
    liveins: $lr
  
    $sp = frame-setup STMDB_UPD $sp, 14, $noreg, killed $r11, killed $lr
    frame-setup CFI_INSTRUCTION def_cfa_offset 8
    frame-setup CFI_INSTRUCTION offset $lr, -4
    frame-setup CFI_INSTRUCTION offset $r11, -8
    $r11 = frame-setup MOVr killed $sp, 14, $noreg, $noreg
    frame-setup CFI_INSTRUCTION def_cfa_register $r11
    $sp = frame-setup SUBri killed $sp, 80, 14, $noreg, $noreg
  
    renamable $r1 = LEApcrel %const.0, 14, $noreg
    renamable $r1 = LDRH killed renamable $r1, $noreg, 0, 14, $noreg :: (load 2 from constant-pool)
    INLINEASM &nop, 1, !4
    renamable $r0 = SPACE 1024, undef renamable $r0
  
    $sp = frame-destroy MOVr $r11, 14, $noreg, $noreg
    $sp = frame-destroy LDMIA_RET $sp, 14, $noreg, def $r11, def $pc, implicit killed $r0

...

________________________________

From: Simon Wallis via Phabricator <reviews at reviews.llvm.org>
Sent: 20 August 2020 14:43
To: Simon Wallis <Simon.Wallis2 at arm.com>; David Green <David.Green at arm.com>; Sam Parker <Sam.Parker at arm.com>; gchatelet at google.com <gchatelet at google.com>; dnsampaio at gmail.com <dnsampaio at gmail.com>
Cc: Kristof Beyls <Kristof.Beyls at arm.com>; Daniel Kiss <Daniel.Kiss at arm.com>; llvm-commits at lists.llvm.org <llvm-commits at lists.llvm.org>; 88888yl at gmail.com <88888yl at gmail.com>; t.p.northover at gmail.com <t.p.northover at gmail.com>; kanheim at a-bix.com <kanheim at a-bix.com>; diana.picus at linaro.org <diana.picus at linaro.org>
Subject: [PATCH] D84923 <https://reviews.llvm.org/D84923>: [ARM] Fix so immediates and pc relative checks

simonwallis2 updated this revision to Diff 286806.
simonwallis2 marked an inline comment as done.
simonwallis2 edited the summary of this revision.
simonwallis2 added a comment.

After review feedback, I removed the change to pc offset estimates from this patch.
This patch is now just about fixing the implementation of SO immediates.

Rewrite long int as int64_t.

CHANGES SINCE LAST ACTION

  https://reviews.llvm.org/D84923/new/

https://reviews.llvm.org/D84923

Files:

  llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
  llvm/test/CodeGen/ARM/constant-island-SOImm.mir

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84923/new/

https://reviews.llvm.org/D84923