llvm dev - May 2008 - [LLVMdev] trouble with 32-bit promotion of return value

I just took a closer read of Chris' "LLVM Exception Handling
Changes"
document and have some comments.

[...]
 > This bloats the code and is quite slow: it's not feasible to use
 > hardware traps to implement these checks.

It's not totally infeasible. An approach similar to what if-conversion
does could implement this. I'm not necessarily advocating this, I'm
just pointing out that there are alternatives.

[...]
 > This is clearly not good.

I can think of a few badnesses, but they could be more clear.

[...]
 > Finally, it is problematic in LLVM for there to be two call  
instructions: one
 > that can throw and one that can't (invoke/call).  Because there are  
two of them,
 > and a lot of code wants to treat them generically, we have classes  
like CallSite
 > that have grown to add a layer of abstraction.  This layer slows  
the compiler
 > down.

This is misleading. It calls out one thing that could be simplified,
while there's no mention of the things that would need to be more
complicated. This proposal may help PEI-dense code, but I wouldn't
guess it'd be an overall compile-time win for PEI-sparse code.

Hmm, and with PEI-dense code, you'd have to watch out for stores to
allocas in between PEIs. mem2reg can't create PHIs that point to
the middles of blocks. Would it be made smart enough to split
blocks when this happens? Do you have any sense of how frequent
this situation is in typical Java or similar code?

[...]
 > Note that this bit is sufficient to represent many possible  
scenarios.  In
 > particular, a Java compiler would mark just about every load, store  
and other
 > exception inducing operation as traping.  If a load is marked  
potentially
 > trapping, the optimizer is required to preserve it (even if its  
value is not
 > used) unless it can prove that it dynamically cannot trap.  In  
practice, this
 > means that standard LLVM analyses would be used to prove that  
exceptions
 > cannot happen, then clear the bit.  As the bits are cleared,  
exception handlers
 > can be deleted and dead loads (for example) can also be removed.
 >
 > This bit also exposes a lot of flexibility and control to the front- 
end.  In
 > particular, gcc supports the -fhonor-snans flag, which would enable  
the trap
 > bit on (basically) all floating point operations.  The presense of  
this flag
 > would trigger generation of the fcomi instruction instead of fucomi  
(on X86)
 > for example, and would inhibit optimizations that would break the  
semantics of
 > the program.

There is no -fhonor-snans; I guess you mean -fsignaling-nans here.

And -fsignaling-nans doesn't mean use fcomi instead of fucomi; the
-ffinite-math-only option does that.

I think you're asking one bit to mean too many different things here.

   "Don't reorder or DCE floating-point operations"

is distinct from

   "Prefer (or don't avoid) instructions that might trap on non-finite
values"

which is distinct yet from

   "Set the processor's control register to enable traps on certain  
exceptions"

Dan

> Hmm, and with PEI-dense code, you'd have to watch out for stores to 
> allocas in between PEIs. mem2reg can't create PHIs that point to the 
> middles of blocks. Would it be made smart enough to split blocks when 
> this happens?
This is what I was attempting to (badly) describe in my previous post,
that there would need to be a second 'flag' to mark modified values
that are accessed in the catch portion...sort-of like a
'write-volatile'.
> Do you have any sense of how frequent this situation is
> in typical Java or similar code?
I have the impression that the most frequent cases don't access any
values modified in the normal execution body.

There is a FIXME part in SelectionDAGLowering::visitRet() that is giving
us trouble. And I would like to know how the community has planned for
addressing this problem.

The current implementation assumes that all C calling conventions
require the return value of integer types in the largest register class
(for most ports 32-bit). However, this is not the case for our port. We
have two register classes (8-bit and 16-bit) and we only return the
8-bit return values in the 8-bit register. The rest is returned in the
memory. But the current behavior of LLVM promotes the return value to 16
bits (largest register class that it can find in the target).

To me, such promotions are Target/ABI specific and should not happen in
the general llvm modules. The correct place (I think) is the target
specific part of the backend. It may also be taken care of in the target
specific modules of the front-end but I'm not sure... (I think it is
more of code generation issue rather than language issue)

Regards
Ali

On May 13, 2008, at 11:54 AM, Alireza.Moshtaghi at microchip.com wrote:
> There is a FIXME part in SelectionDAGLowering::visitRet() that is  
> giving
> us trouble. And I would like to know how the community has planned for
> addressing this problem.
>
> The current implementation assumes that all C calling conventions
> require the return value of integer types in the largest register  
> class
> (for most ports 32-bit). However, this is not the case for our port.  
> We
> have two register classes (8-bit and 16-bit) and we only return the
> 8-bit return values in the 8-bit register. The rest is returned in the
> memory. But the current behavior of LLVM promotes the return value  
> to 16
> bits (largest register class that it can find in the target).
>
> To me, such promotions are Target/ABI specific and should not happen  
> in
> the general llvm modules. The correct place (I think) is the target
> specific part of the backend. It may also be taken care of in the  
> target
> specific modules of the front-end but I'm not sure... (I think it is
> more of code generation issue rather than language issue)

I think a reasonable way to fix this would be to add a new field to
the TargetLowering class, and then let each target specify the minimum
integer type to promote return types to. SelectionDAGISel.cpp can then
use this information instead of hard-coding i32. You can have the
default value be i32 so that most targets don't need override it.

Dan