[llvm] 1420f4e - [AVR] Fix I/O instructions on XMEGA

Mikael Holmén via llvm-commits llvm-commits at lists.llvm.org
Mon May 18 23:32:41 PDT 2020


Hi Vlastimil and Dylan,

With gcc we see a few new warnings with this commit:

In file included from ../lib/Target/AVR/AVRISelDAGToDAG.cpp:47:0:
lib/Target/AVR/AVRGenDAGISel.inc: In member function 'virtual bool
llvm::AVRDAGToDAGISel::CheckNodePredicate(llvm::SDNode*, unsigned int)
const':
lib/Target/AVR/AVRGenDAGISel.inc:1264:14: warning: comparison of
unsigned expression >= 0 is always true [-Wtype-limits]
   return val >= 0x0 && val < 0x20;
          ~~~~^~~~~~
lib/Target/AVR/AVRGenDAGISel.inc:1323:14: warning: comparison of
unsigned expression >= 0 is always true [-Wtype-limits]
   return val >= 0x0 && val < 0x40;
          ~~~~^~~~~~
lib/Target/AVR/AVRGenDAGISel.inc:1351:14: warning: comparison of
unsigned expression >= 0 is always true [-Wtype-limits]
   return val >= 0x0 && val < 0x3f;
          ~~~~^~~~~~

Is it perhaps ok to just remove the three "val >= 0" comparisons above
to get rid of the warnings?

/Mikael

On Sun, 2020-05-17 at 00:47 -0700, Dylan McKay via llvm-commits wrote:
> Author: Dylan McKay
> Date: 2020-05-17T19:46:09+12:00
> New Revision: 1420f4efbe7ca34355b2dd85d396197d92cd5e3f
> 
> URL: 
> https://protect2.fireeye.com/v1/url?k=2f28a438-71881e56-2f28e4a3-866038973a15-e2e1a973d1d73ac6&q=1&e=816ce472-1ef3-4749-930f-6424e45b152f&u=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Fcommit%2F1420f4efbe7ca34355b2dd85d396197d92cd5e3f
> DIFF: 
> https://protect2.fireeye.com/v1/url?k=c7ad39e1-990d838f-c7ad797a-866038973a15-68575abf43763fdf&q=1&e=816ce472-1ef3-4749-930f-6424e45b152f&u=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Fcommit%2F1420f4efbe7ca34355b2dd85d396197d92cd5e3f.diff
> 
> LOG: [AVR]  Fix I/O instructions on XMEGA
> 
> Summary:
> On XMEGA, I/O address space is same as data address space - there is
> no 0x20 offset,
> because CPU General Purpose Registers are not mapped in data address
> space.
> 
> > From https://en.wikipedia.org/wiki/AVR_microcontrollers
> > In the XMEGA variant, the working register file is not mapped into
> > the data address space; as such, it is not possible to treat any of
> > the XMEGA's working registers as though they were SRAM. Instead,
> > the I/O registers are mapped into the data address space starting
> > at the very beginning of the address space.
> 
> Reviewers: dylanmckay
> 
> Reviewed By: dylanmckay
> 
> Subscribers: hiraditya, Jim, llvm-commits
> 
> Tags: #llvm
> 
> Differential Revision: https://reviews.llvm.org/D77207
> 
> Patch by Vlastimil Labsky.
> 
> Added: 
>     llvm/test/CodeGen/AVR/features/xmega_io.ll
> 
> Modified: 
>     llvm/lib/Target/AVR/AVRDevices.td
>     llvm/lib/Target/AVR/AVRInstrInfo.td
>     llvm/lib/Target/AVR/AVRSubtarget.cpp
>     llvm/lib/Target/AVR/AVRSubtarget.h
> 
> Removed: 
>     
> 
> 
> #####################################################################
> ###########
> diff  --git a/llvm/lib/Target/AVR/AVRDevices.td
> b/llvm/lib/Target/AVR/AVRDevices.td
> index 62def4574437..6730f2e1673e 100644
> --- a/llvm/lib/Target/AVR/AVRDevices.td
> +++ b/llvm/lib/Target/AVR/AVRDevices.td
> @@ -121,6 +121,11 @@ def FeatureTinyEncoding   :
> SubtargetFeature<"tinyencoding",
>                                    "The device has Tiny core specific
> "
>                                    "instruction encodings">;
>  
> +// The device has CPU registers mapped in data address space
> +def FeatureMMR : SubtargetFeature<"memmappedregs",
> "m_hasMemMappedGPR",
> +                                  "true", "The device has CPU
> registers "
> +                                  "mapped in data address space">;
> +
>  class ELFArch<string name>  : SubtargetFeature<"", "ELFArch",
>                                      !strconcat("ELF::",name), "">;
>  
> @@ -152,7 +157,7 @@ def ELFArchXMEGA7  :
> ELFArch<"EF_AVR_ARCH_XMEGA7">;
>  // device should have.
>  def FamilyAVR0           : Family<"avr0", []>;
>  
> -def FamilyAVR1           : Family<"avr1", [FamilyAVR0, FeatureLPM]>;
> +def FamilyAVR1           : Family<"avr1", [FamilyAVR0, FeatureLPM,
> FeatureMMR]>;
>  
>  def FamilyAVR2           : Family<"avr2",
>                                   [FamilyAVR1, FeatureIJMPCALL,
> FeatureADDSUBIW,
> @@ -190,11 +195,14 @@ def FamilyAVR6           : Family<"avr6",
>  
>  def FamilyTiny           : Family<"avrtiny",
>                                   [FamilyAVR0, FeatureBREAK,
> FeatureSRAM,
> -                                  FeatureTinyEncoding]>;
> +                                  FeatureTinyEncoding, FeatureMMR]>;
>  
>  def FamilyXMEGA          : Family<"xmega",
> -                                 [FamilyAVR51, FeatureEIJMPCALL,
> FeatureSPMX,
> -                                  FeatureDES]>;
> +                                 [FamilyAVR0, FeatureLPM,
> FeatureIJMPCALL, FeatureADDSUBIW,
> +                                  FeatureSRAM, FeatureJMPCALL,
> FeatureMultiplication,
> +                                  FeatureMOVW, FeatureLPMX,
> FeatureSPM,
> +                                  FeatureBREAK, FeatureEIJMPCALL,
> FeatureSPMX,
> +                                  FeatureDES, FeatureELPM,
> FeatureELPMX]>;
>  
>  def FamilyXMEGAU         : Family<"xmegau",
>                                    [FamilyXMEGA, FeatureRMW]>;
> @@ -208,7 +216,7 @@ def FeatureSetSpecial    : FeatureSet<"special",
>                                         FeatureLPM, FeatureLPMX,
> FeatureELPM,
>                                         FeatureELPMX, FeatureSPM,
> FeatureSPMX,
>                                         FeatureDES, FeatureRMW,
> -                                       FeatureMultiplication,
> FeatureBREAK]>;
> +                                       FeatureMultiplication,
> FeatureBREAK, FeatureMMR]>;
>  
>  //===---------------------------------------------------------------
> ------===//
>  // AVR microcontrollers supported.
> 
> diff  --git a/llvm/lib/Target/AVR/AVRInstrInfo.td
> b/llvm/lib/Target/AVR/AVRInstrInfo.td
> index 55a173573c14..5012ddfa7af4 100644
> --- a/llvm/lib/Target/AVR/AVRInstrInfo.td
> +++ b/llvm/lib/Target/AVR/AVRInstrInfo.td
> @@ -107,7 +107,9 @@ def imm_com8 : Operand<i8> {
>  
>  def ioaddr_XFORM : SDNodeXForm<imm,
>  [{
> -  return CurDAG->getTargetConstant(uint8_t(N->getZExtValue()) -
> 0x20, SDLoc(N), MVT::i8);
> +  uint8_t offset = Subtarget->getIORegisterOffset();
> +  return CurDAG->getTargetConstant(uint8_t(N->getZExtValue()) -
> offset,
> +                                   SDLoc(N), MVT::i8);
>  }]>;
>  
>  def iobitpos8_XFORM : SDNodeXForm<imm,
> @@ -124,20 +126,23 @@ def iobitposn8_XFORM : SDNodeXForm<imm,
>  
>  def ioaddr8 : PatLeaf<(imm),
>  [{
> -  uint64_t val = N->getZExtValue();
> -  return val >= 0x20 && val < 0x60;
> +  uint8_t offset = Subtarget->getIORegisterOffset();
> +  uint64_t val = N->getZExtValue() - offset;
> +  return val >= 0x0 && val < 0x40;
>  }], ioaddr_XFORM>;
>  
>  def lowioaddr8 : PatLeaf<(imm),
>  [{
> -  uint64_t val = N->getZExtValue();
> -  return val >= 0x20 && val < 0x40;
> +  uint8_t offset = Subtarget->getIORegisterOffset();
> +  uint64_t val = N->getZExtValue() - offset;
> +  return val >= 0x0 && val < 0x20;
>  }], ioaddr_XFORM>;
>  
>  def ioaddr16 : PatLeaf<(imm),
>  [{
> -  uint64_t val = N->getZExtValue();
> -  return val >= 0x20 && val < 0x5f;
> +  uint8_t offset = Subtarget->getIORegisterOffset();
> +  uint64_t val = N->getZExtValue() - offset;
> +  return val >= 0x0 && val < 0x3f;
>  }], ioaddr_XFORM>;
>  
>  def iobitpos8 : PatLeaf<(imm),
> 
> diff  --git a/llvm/lib/Target/AVR/AVRSubtarget.cpp
> b/llvm/lib/Target/AVR/AVRSubtarget.cpp
> index bd4a3fcb5fcd..195ca95bc3bd 100644
> --- a/llvm/lib/Target/AVR/AVRSubtarget.cpp
> +++ b/llvm/lib/Target/AVR/AVRSubtarget.cpp
> @@ -29,16 +29,16 @@ namespace llvm {
>  
>  AVRSubtarget::AVRSubtarget(const Triple &TT, const std::string &CPU,
>                             const std::string &FS, const
> AVRTargetMachine &TM)
> -    : AVRGenSubtargetInfo(TT, CPU, FS),
> -      ELFArch(0),
> +    : AVRGenSubtargetInfo(TT, CPU, FS), ELFArch(0),
>  
>        // Subtarget features
>        m_hasSRAM(false), m_hasJMPCALL(false), m_hasIJMPCALL(false),
>        m_hasEIJMPCALL(false), m_hasADDSUBIW(false),
> m_hasSmallStack(false),
> -      m_hasMOVW(false), m_hasLPM(false),
> m_hasLPMX(false),  m_hasELPM(false),
> +      m_hasMOVW(false), m_hasLPM(false), m_hasLPMX(false),
> m_hasELPM(false),
>        m_hasELPMX(false), m_hasSPM(false), m_hasSPMX(false),
> m_hasDES(false),
>        m_supportsRMW(false), m_supportsMultiplication(false),
> m_hasBREAK(false),
> -      m_hasTinyEncoding(false), m_FeatureSetDummy(false),
> +      m_hasTinyEncoding(false), m_hasMemMappedGPR(false),
> +      m_FeatureSetDummy(false),
>  
>        InstrInfo(), FrameLowering(),
>        TLInfo(TM, initializeSubtargetDependencies(CPU, FS, TM)),
> TSInfo() {
> 
> diff  --git a/llvm/lib/Target/AVR/AVRSubtarget.h
> b/llvm/lib/Target/AVR/AVRSubtarget.h
> index ca4167fcb335..81d883eb30d9 100644
> --- a/llvm/lib/Target/AVR/AVRSubtarget.h
> +++ b/llvm/lib/Target/AVR/AVRSubtarget.h
> @@ -71,6 +71,9 @@ class AVRSubtarget : public AVRGenSubtargetInfo {
>    bool supportsMultiplication() const { return
> m_supportsMultiplication; }
>    bool hasBREAK() const { return m_hasBREAK; }
>    bool hasTinyEncoding() const { return m_hasTinyEncoding; }
> +  bool hasMemMappedGPR() const { return m_hasMemMappedGPR; }
> +
> +  uint8_t getIORegisterOffset() const { return hasMemMappedGPR() ?
> 0x20 : 0x0; }
>  
>    /// Gets the ELF architecture for the e_flags field
>    /// of an ELF object file.
> @@ -105,6 +108,7 @@ class AVRSubtarget : public AVRGenSubtargetInfo {
>    bool m_supportsMultiplication;
>    bool m_hasBREAK;
>    bool m_hasTinyEncoding;
> +  bool m_hasMemMappedGPR;
>  
>    // Dummy member, used by FeatureSet's. We cannot have a
> SubtargetFeature with
>    // no variable, so we instead bind pseudo features to this
> variable.
> 
> diff  --git a/llvm/test/CodeGen/AVR/features/xmega_io.ll
> b/llvm/test/CodeGen/AVR/features/xmega_io.ll
> new file mode 100644
> index 000000000000..713b2dec346a
> --- /dev/null
> +++ b/llvm/test/CodeGen/AVR/features/xmega_io.ll
> @@ -0,0 +1,48 @@
> +; RUN: llc -O0 < %s -march=avr -mcpu avrxmega1 | FileCheck %s
> -check-prefix=XMEGA
> +; RUN: llc -O0 < %s -march=avr -mcpu avrxmega2 | FileCheck %s
> -check-prefix=XMEGA
> +; RUN: llc -O0 < %s -march=avr -mcpu avrxmega3 | FileCheck %s
> -check-prefix=XMEGA
> +; RUN: llc -O0 < %s -march=avr -mcpu avrxmega4 | FileCheck %s
> -check-prefix=XMEGA
> +; RUN: llc -O0 < %s -march=avr -mcpu avrxmega5 | FileCheck %s
> -check-prefix=XMEGA
> +; RUN: llc -O0 < %s -march=avr -mcpu avrxmega6 | FileCheck %s
> -check-prefix=XMEGA
> +; RUN: llc -O0 < %s -march=avr -mcpu avrxmega7 | FileCheck %s
> -check-prefix=XMEGA
> +; RUN: llc -O0 < %s -march=avr -mcpu avr2 | FileCheck %s -check-
> prefix=AVR
> +; RUN: llc -O0 < %s -march=avr -mcpu avr25 | FileCheck %s -check-
> prefix=AVR
> +; RUN: llc -O0 < %s -march=avr -mcpu avr3 | FileCheck %s -check-
> prefix=AVR
> +; RUN: llc -O0 < %s -march=avr -mcpu avr31 | FileCheck %s -check-
> prefix=AVR
> +; RUN: llc -O0 < %s -march=avr -mcpu avr35 | FileCheck %s -check-
> prefix=AVR
> +; RUN: llc -O0 < %s -march=avr -mcpu avr4 | FileCheck %s -check-
> prefix=AVR
> +; RUN: llc -O0 < %s -march=avr -mcpu avr5 | FileCheck %s -check-
> prefix=AVR
> +; RUN: llc -O0 < %s -march=avr -mcpu avr51 | FileCheck %s -check-
> prefix=AVR
> +; RUN: llc -O0 < %s -march=avr -mcpu avr6 | FileCheck %s -check-
> prefix=AVR
> +
> +define i8 @read8_low_io() {
> +; CHECK-LABEL: read8_low_io
> +; XMEGA: in r24, 8
> +; AVR: lds r24, 8
> +  %1 = load i8, i8* inttoptr (i16 8 to i8*)
> +  ret i8 %1
> +}
> +
> +define i8 @read8_hi_io() {
> +; CHECK-LABEL: read8_hi_io
> +; XMEGA: in r24, 40
> +; AVR: in r24, 8
> +  %1 = load i8, i8* inttoptr (i16 40 to i8*)
> +  ret i8 %1
> +}
> +
> +define i8 @read8_maybe_io() {
> +; CHECK-LABEL: read8_maybe_io
> +; XMEGA: lds r24, 80
> +; AVR: in r24, 48
> +  %1 = load i8, i8* inttoptr (i16 80 to i8*)
> +  ret i8 %1
> +}
> +
> +define i8 @read8_not_io(){
> +; CHECK-LABEL: read8_not_io
> +; XMEGA: lds r24, 160
> +; AVR: lds r24, 160
> +  %1 = load i8, i8* inttoptr (i16 160 to i8*)
> +  ret i8 %1
> +}
> 
> 
>         
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list