[PATCH] D36772: Unmerge GEPs to reduce register pressure on IndirectBr edges.

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:6264
+    UGEPI->setOperand(1, NewUGEPIIdx);
+  }
+  // After unmerging, verify that GEPIOp is actually only used in SrcBlock (not
----------------
yamauchi wrote:
> hfinkel wrote:
> > You'll also need to make sure that this GEP is not marked as inbounds if GEPI was not.
> > 
> >   if (!GEPI->isInBounds()) {
> >     UGEPI->setIsInBounds(false);
> >   }
> > 
> > because otherwise the result of GEP could be not-in-bounds resulting in UB if that's used as the input to an inbounds UGEPI.
> I'm not very familiar with how inbounds works.
> 
> Is an inbounds GEP UB if it takes a non-inbounds GEP as its operand (regardless of whether the index/offset is actually in bounds or not)?
> 
> For example,
> 
> Before:
> 
> %GEPIOp = ...
> %GEPI = gep %GEPIOp 2
> %UGEPI = gep inbounds %GEPIOp 1
> 
> After:
> 
> %GEPIOp = ...
> %GEPI = gep %GEPIOp 2
> %UGEPI = gep inbounds %GEPI -1
> 
> Suppose "gep %GEPIOp 2" is not in bounds and "gep inbounds %GEPIOp 1" is in bounds. Both aren't UB.
> 
> "gep inbounds %GEPI -1" is UB just because it takes (non-inbounds) "gep %GEPIOp 2" as an operand, even though the offset/index of "gep inbounds %GEPI -1" is actually in bounds?
> 
Yes, the base pointer needs to be inbounds too. The LangRef says, "If the inbounds keyword is present, the result value of the getelementptr is a poison value if the base pointer is not an in bounds address of an allocated object, or if any of the addresses that would be formed by successive addition of the offsets implied by the indices to the base address with infinitely precise signed arithmetic are not an in bounds address of that allocated object." That's exactly why you need to account for the inbounds here.


https://reviews.llvm.org/D36772