[PATCH] D142107: [AVR] Support address space casts

[Ben Shi via Phabricator via llvm-commits]

benshi001 added inline comments.


================
Comment at: llvm/lib/Target/AVR/AVRTargetMachine.h:57
+    // pointers.
+    return getPointerSize(SrcAs) == getPointerSize(DestAs);
+  }
----------------
aykevl wrote:
> benshi001 wrote:
> > it would be better to also check the range, something like
> > 
> > ```
> >    return SrcAs < AVR::NumAddrSpaces && DestAs < AVR::NumAddrSpaces &&
> >           getPointerSize(SrcAs) == getPointerSize(DestAs);
> > ```
> Please take a look at other architecture. Most of them make this a no-op without constraints, like here on ARM:
> 
> ```
>   /// Returns true if a cast between SrcAS and DestAS is a noop.
>   bool isNoopAddrSpaceCast(unsigned SrcAS, unsigned DestAS) const override {
>     // Addrspacecasts are always noops.
>     return true;
>   }
> ```
> 
> Why would AVR be different?
It is just a suggestion, since I find

```
bool X86TargetMachine::isNoopAddrSpaceCast(unsigned SrcAS,
                                           unsigned DestAS) const {
  assert(SrcAS != DestAS && "Expected different address spaces!");
  if (getPointerSize(SrcAS) != getPointerSize(DestAS))
    return false;
  return SrcAS < 256 && DestAS < 256;
}
```

Is there some possibility that AVR's addrspace will exceed `AVR::NumAddrSpaces` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142107