On Jan 17, 2008, at 10:03 PM, Duncan Sands wrote:
> Hi Chris,
>> + LegalizeAction getTruncStoreAction(MVT::ValueType ValVT,
>> + MVT::ValueType MemVT) const {
>> + assert(ValVT < MVT::LAST_VALUETYPE && MemVT < 32 &&
>
> what is 32? Did you mean <= LAST_INTEGER_VALUETYPE or something?
> Now I
> come to notice it, it is bad that you have to use < for
> LAST_VALUE_TYPE
> but <= for LAST_INTEGER_VALUETYPE and friends...
32 is sizeof(uint64_t)*8 / 2, because each entry takes two bits in a
uint64_t.
I converted this to symbolic math to make it more obvious, and
switched to using array_lengthof instead of comparing against
MVT::LAST_VALUETYPE directly.
>> - /// isStoreXLegal - Return true if the specified store with
>> truncation is
>> + /// isTruncStoreLegal - Return true if the specified store with
>> truncation is
>> /// legal on this target.
>> - bool isStoreXLegal(MVT::ValueType VT) const {
>> - return getStoreXAction(VT) == Legal || getStoreXAction(VT) ==
>> Custom;
>> + bool isTruncStoreLegal(MVT::ValueType ValVT, MVT::ValueType
>> MemVT) const {
>> + return getTruncStoreAction(ValVT, MemVT) == Legal ||
>> + getTruncStoreAction(ValVT, MemVT) == Custom;
>> }
>
> It would be more friendly to have isStoreXLegal return false if VT
> or MemVT is
> an extended value type, rather than having it assert in
> getTruncStoreAction...
This is tricky because I can't just use isTypeLegal here. PPC for
example supports a truncstore from i32 to i8 even though i8 isn't
legal. When I return to working on apints, I'll figure out what the
right answer is.
>> + // If this is an FP_ROUND or TRUNC followed by a store, fold
>> this into a
>> + // truncating store. We can do this even if this is already a
>> truncstore.
>> + if ((Value.getOpcode() == ISD::FP_ROUND || Value.getOpcode() ==
>> ISD::TRUNCATE)
>> + && TLI.isTypeLegal(Value.getOperand(0).getValueType()) &&
>> + Value.Val->hasOneUse() && ST->isUnindexed() &&
>> + TLI.isTruncStoreLegal(Value.getOperand(0).getValueType(),
>> + ST->getStoredVT())) {
>
> For example, this will assert if ST->getStoredVT() >= 32, eg an
> apfloat (!). Also,
> before legalize this could be done whether all these guys are legal
> or not I suppose.
It could be done, but isn't profitable and doesn't simplify the code
necessarily.
>> + case Expand:
>> + // Just store the low part. This may become a non-trunc
>> store, so make
>> + // sure to use getTruncStore, not UpdateNodeOperands below.
>> + ExpandOp(ST->getValue(), Tmp3, Tmp4);
>> + return DAG.getTruncStore(Tmp1, Tmp3, Tmp2, ST-
>> >getSrcValue(),
>> + SVOffset, MVT::i8, isVolatile,
>> Alignment);
>
> This may be wrong for apints, since the expanded value type may be
> smaller than
> ST->getStoredVT(), eg if you are expanding i64, and ST-
> >getStoredVT() is i40. I
> can take care of this.
Ok. As it turns out, I think this code is unreachable currently. If
we made the above dag combine xform happen even for illegal xforms
before legalize, then this code would be live.
-Chris
_______________________________________________
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
